Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Element status labels #14968

Merged
merged 10 commits into from
May 22, 2024
Merged

Conversation

brandonkelly
Copy link
Member

@brandonkelly brandonkelly commented May 9, 2024

Adds a new “Status” table attribute to element indexes, shown by default for new element sources, which outputs the element’s status + label.

When shown, element chips in the first column no longer include status indicators.

An entry index in table view, with the “Status” attribute enabled.

Element cards have also been updated to always show the full status labels rather than just the indicators.

An entry index in card view, showing that each card includes the new element status label below the normal card content.

Copy link

linear bot commented May 9, 2024

Revert the Element::getCardLabelHtml() change, and move status label below other card content
It’s already going to get re-rendered automatically via Craft.refreshElementInstances().

Fixes a bug where the status indicator would come back when it shouldn’t since there’s a Status column
# Conflicts:
#	src/web/assets/cp/dist/cp.js
#	src/web/assets/cp/dist/cp.js.map
#	src/web/assets/cp/dist/css/cp.css
#	src/web/assets/cp/dist/css/cp.css.map
Copy link
Contributor

@gcamacho079 gcamacho079 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Wherever there is adjacent text, the img role and aria-label can be removed from the .status element.
  • When .sel class is applied to elements, all status label text drops under the 4.5:1 minimum contrast ratio.
  • I noticed a couple index pages that still showed the indicator-only status: a custom source and a channel. Not sure if it's something I did on my end, or if there's something unique about these views that's causing them to show the previous status indicator.
  • The text is very small and also uses px for sizing. I recommend we use rem to allow font resizing, and bump up the font size a bit for better readability.
  • "Use shapes for statuses" do not affect the indicator shape in the status label. I think it would be a great idea to also include shape in the default status labels. From the Carbon Design System docs for status indicators: "Do use shape, color, and status labels to improve scannability. Avoid using only color and status labels to differentiate your content."
  • There were a couple places where I noticed the adjacent text hasn't been implemented:
    • Notification toasts
    • Breadcrumbs
    • Author chips on edit screens
    • "Similar issues on GitHub" part of Support widget

@gcamacho079 gcamacho079 added accessibility 👤 features related to accessibility 🖥 UI labels May 17, 2024
# Conflicts:
#	src/web/assets/cp/dist/cp.js
#	src/web/assets/cp/dist/cp.js.map
#	src/web/assets/cp/dist/css/cp.css
#	src/web/assets/cp/dist/css/cp.css.map
@brandonkelly
Copy link
Member Author

I noticed a couple index pages that still showed the indicator-only status: a custom source and a channel. Not sure if it's something I did on my end, or if there's something unique about these views that's causing them to show the previous status indicator.

The “Status” column is enabled by default for native element sources of built-in element types, but developers will need to explicitly opt into showing it if they’ve already saved custom source settings. And custom sources don’t make any assumptions about which columns you are going to want; that is always chosen by the developer when defining the custom source via the “Default Table Columns” setting.

If the Status column isn’t visible, we will continue to show the status indicators (sans text label) within the element chips.

The text is very small and also uses px for sizing. I recommend we use rem to allow font resizing, and bump up the font size a bit for better readability.

Increased to 11px, which matches the source headings. None of the other font sizes use rems yet; I’ll let Brian be the one to make that change across the board when the time comes.

There were a couple places where I noticed the adjacent text hasn't been implemented:

Yeah as I said this change only applies to element index tables (when a “Status” column is included) and element cards. We can discuss including them in element chips down the road (probably via an accessibility setting), but for now I don’t think we should hold back on an improvement up just because it’s not a complete solution.

@brandonkelly brandonkelly merged commit 1938d17 into 5.2 May 22, 2024
5 checks passed
@brandonkelly brandonkelly deleted the feature/pt-24-redesign-status-indicator branch May 22, 2024 23:40
brandonkelly added a commit that referenced this pull request May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility 👤 features related to accessibility 🖥 UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants