Problem/Motivation

I was having hard time finding the more actions button. Turns out I can't really see it because for some reason the icon is not visible:

Steps to reproduce

  • Open latest version of Chrome (currently using v133.0.6943.100 at time of writing)
  • Login as admin
  • Turn on navigation and navigation_top_bar modules
  • Create and edit content, or just edit an existing piece of content

Expected behavior

Top bar displays with a "More Actions" button on the right-hand side that is denoted by a three-dot icon, eg.

Screenshot of expected behavior

Actual behavior

Top bar displays, but the "More Actions" button on the right-hand side, while present, does not show the three-dot icon, eg.

Screenshot of actual behavior

Proposed resolution

In Chrome, the ::before pseudo element supposed to render for this icon is not. The issue appears to be that no content property is calculated. This is set with the following CSS rule:

https://git.drupalcode.org/project/drupal/-/blob/11.1.x/core/modules/nav...

data-icon-text is not defined for this icon b/c its icon only, it doesn't have a corresponding text label. It should at least have some hidden text for screen readers. That wouldn't resolve our issue here though.

To resolve, lets add an additional modifier to the toolbar-button component, icon-only, which explicitly sets content: ''. In this way, we avoid Chromes' seeming issue in dealing with content: attr(data-icon-text) when data-icon-text is undefined.

Remaining tasks

  • MR
  • Review

User interface changes

Three dot icon shows when needed.

CommentFileSizeAuthor
#7 actual.png343.48 KBm4olivei
#7 expected.png345.39 KBm4olivei
Screenshot 2025-02-07 at 1.39.49 PM.png8.66 KBlauriii

Issue fork drupal-3505124

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

lauriii created an issue. See original summary.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

Debugged this with @plopesc and this looks like a regression in Chrome in version 133. 😭

quietone’s picture

quietone commented
Version: 11.1.x-dev » 11.x-dev
kentr’s picture

kentr commented

How does one reproduce this?

finnsky’s picture

finnsky commented
Status: Active » Postponed

We need to pause icons tickets and concentrate on Icon API implementation

https://www.drupal.org/project/drupal/issues/3483209

finnsky’s picture

finnsky commented

It is not small hot fix but reason is in wrong icons management in module.

https://gyazo.com/cfbb9da933c5d883bf840b02a9b6a522

All buttons with only one icon broken. Close on mobile as well.
I would like to fix that system problem in global linked ticket.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Assigned: Unassigned » m4olivei
Issue summary: View changes
Status: Postponed » Active
Issue tags: +Navigation stable blocker
StatusFileSize
new expected.png345.39 KB
new actual.png343.48 KB

I found that the issue appears to be Chrome's inability to interpret content: attr(data-icon-text); when the data-icon-text attribute is not set. We don't have any text label for this particular button, so I'm proposing that we add a new modifier, icon-only to the toolbar-button component, so we can style it explicitly as content: ''. That will make the ::before pseudo element appear as expected and fix the issue reported here.

This is a quick fix that we should be able to get in quickly before the icon management lands.

Updated issue summary. Marking as active. Will work on an MR.

m4olivei opened merge request !11287

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Assigned: m4olivei » Unassigned
Issue summary: View changes
Status: Active » Needs review
finnsky’s picture

finnsky commented
Status: Needs review » Needs work

1. I prefer not to duplicate class functionality. We already have [class*="toolbar-button--icon"]
We should use it. Just add a check for the text attribute.

2. In #6 I pointed out that the close button on mobile is also broken.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs work » Needs review

1. I prefer not to duplicate class functionality. We already have [class*="toolbar-button--icon"]
We should use it. Just add a check for the text attribute.

Good point! I've got rid of the extra modifier and class, and instead used a [class*="toolbar-button--icon"]:not([data-icon-text])::before selector. Is that what you meant, or did you have something else in mind?

2. In #6 I pointed out that the close button on mobile is also broken.

Sorry I missed this. I didn't know what you meant, b/c I couldn't see the icon, because it was broken 😆. The above selector catches the Close icon and fixes it as well.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Verified the problem following the steps provided in the IS.

Applying the MR did fix the issue, not uploading a screenshot as they are already included.

Javascript failure was random

finnsky’s picture

finnsky commented
Status: Reviewed & tested by the community » Closed (outdated)

Fixed in related issue.
https://www.drupal.org/project/drupal/issues/3483209