Use warning icon for obsolete and deprecated modules on admin/modules in Claro

Created on 18 September 2021, over 3 years ago
Updated 20 July 2024, 5 months ago

Problem/Motivation

After #3215043: Indicate the non-stable statuses in admin/modules page and #3225812: Add lifecycle_link key to info.yml files the Seven theme adds a warning icon to /admin/modules for any obsolete or deprecated module that provides lifecycle_link in its .info.yml file. See also the change records

The Claro theme should indicate obsolete and deprecated modules as well.

Steps to reproduce

Add the following lines to core/modules/forum/forum.info.yml:

lifecycle: deprecated
lifecycle_link: http://example.com

With that addition and the patch from #3215043, the Forum module shows up like this in the Seven theme:

Proposed resolution

Remaining tasks

User interface changes

We have restored the warning icon before the deprecated text on deprecated modules, just like in the Seven theme. Please refer to the after screenshot for reference.

Before:

After:

API changes

None

Data model changes

None

Release notes snippet

None

📌 Task
Status

Needs work

Version

11.0 🔥

Component
Claro 

Last updated 2 days ago

Created by

🇺🇸United States benjifisher Boston area

Live updates comments and jobs are added and updated live.
  • Needs manual testing

    The change/bugfix cannot be fully demonstrated by automated testing, and thus requires manual testing in a variety of environments.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,367 pass
  • 🇮🇳India gauravvvv Delhi, India

    I have added a warning icon to highlight the deprecated modules same as seven theme. please review

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    Issue summary appears to be incomplete.

  • Status changed to Needs review 8 months ago
  • 🇮🇳India gauravvvv Delhi, India

    After Merge Request !7753

  • Status changed to Needs work 8 months ago
  • The Needs Review Queue Bot tested this issue.

    While you are making the above changes, we recommend that you convert this patch to a merge request . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

  • Pipeline finished with Success
    8 months ago
    Total: 587s
    #156987
  • Status changed to Needs review 8 months ago
  • 🇮🇳India gauravvvv Delhi, India
  • Status changed to RTBC 8 months ago
  • 🇮🇳India Vishal Choudhary Dharmshala

    "I have reviewed this deprecated text, and it highlights much better now. It's fine to consider any user's perspective."
    So finally We Move to RTBC.
    Thanks.

  • Status changed to Needs review 8 months ago
  • 🇺🇸United States xjm

    #3215043: Indicate the non-stable statuses in admin/modules page seems to be removing this exact CSS from a number of places, so I'm a bit confused. Is this issue even still necessary? Why doesn't the original solution work in Claro? Can we re-test to confirm that HEAD is missing it?

  • 🇺🇸United States xjm

    In #16 I was looking at the patch. Hiding that. Question still stands as to why this is necessary/whether this is the correct approach though.

  • 🇺🇸United States xjm
  • Status changed to RTBC 8 months ago
  • 🇫🇮Finland simohell

    I reviewed this issue, that is a follow-up on #3215043: Indicate the non-stable statuses in admin/modules page .

    This MR adds an existing icon from the theme in front of the link with text "(Deprecated)". This icon used is the correct one and helps user to identify that using a deprecated module carries some risk. Icon is added as a background image and does not have effect on Voiceover screenreader.

    The confirmation message does not need the additional icon as the whole message is styled as a warning that has it's own icon.

  • Status changed to Needs work 8 months ago
  • 🇺🇸United States xjm

    @simohell:

    • Manual testing is required to prove that the bug still exists. The "followup" you mention was committed years ago and therefore may have already resolved this.
    • My questions in #16 also need to be addressed
    • Either a Claro maintainer or (preferably) a frontend framework manager needs to validate the approach.
  • 🇫🇮Finland simohell

    @xjm You mean the main issue is, that 11.x does not include /modules/system/css/system.admin.css on admin pages on Claro (or Olivero for that matter) even if it should? And it is also not certain wether it should or not?

  • 🇫🇮Finland simohell

    Including /core/modules/system/css/system.admin.css would break some of the styling in Claro, so it makes sense that same icon is added to Claro in it's own CSS. Attached a screenshot of what would happend if the system CSS file would be used. #12 shows the expected result.

    Claro with system.admin.css applied:

  • 🇺🇸United States xjm

    Again, what I'm looking for is Claro tested from the current 11.x or 10.3.x HEAD, without any fix applied.

  • 🇮🇳India gauravvvv Delhi, India

    Here is before patch screenshot, without any fix applied on 11.x

  • 🇫🇮Finland simohell

    Sorry. I had both but comment had 11.x HEAD only as attachement, not inline.

  • Status changed to Needs review 8 months ago
  • 🇫🇮Finland simohell

    From #20 the task "Either a Claro maintainer or (preferably) a frontend framework manager needs to validate the approach." I think falls under the definition of "review" so I'll tag this back to "needs review".

  • 🇬🇧United Kingdom catch

    I think this needs review from a Claro maintainer - there's some overlap with FEFMs anyway.

  • Status changed to Needs work 5 months ago
  • 🇪🇸Spain ckrina Barcelona

    First, thanks all for working on this and finding a place that needs improvement. I agree with you all on this being one of those places.

    We have restored the warning icon before the deprecated text on deprecated modules, just like in the Seven theme.

    Even if this existed in Seven, it doesn't mean it'll visually work in Claro: it results in a really packed, confusing and un-structured group of elements. The solution proposed here is not the right one on a design/UI perspective. The title/heading is designed to hold the title, not title + parenthesis and its text + icons. We can't add info and elements to places that are not designed for that when there are no designs.

    The goal of this issue is to make more obvious that a module is obsolete. +1 to that. The next step here is to find the right UI solution for Claro (applies to any UI), and that needs a designer designing that new part of the UI that don't exists (even if that existed in Seven, that solution doesn't work here). When that design is aproved, then let's jump into the implementation. This is the best workflow to be sure we end up with digestible UIs. :)

Production build 0.71.5 2024