Theming is currently a bit difficult because certain styles are handled via JS, whereas it'd be a lot easier if the styles were handled via CSS.

When using the default Gin toolbar setting—"Sidebar, Vertical Toolbar"—the toolbar can be either open, showing the icons and text labels for each menu item, or closed, showing only the icons. The toolbar can be opened or closed according to the user's preference using the open/close button. I believe the default initial state is open.

Gin Vertical Toolbar WITHOUT Environment Indicator

If Environment Indicator is not enabled, when a user hovers one of the main items of an open toolbar, that item's sub-menu appears offset to the right by an amount that accounts for the width of the open toolbar, leaving the icons and text labels fully visible and available to hover or click as illustrated below:

Gin without Environment Indicator

(EDIT: The behavior described above was introduced in Gin 3.0-rc1. In previous versions, sub-menus did overlay the main toolbar item text labels.)

Gin Vertical Toolbar WITH Environment Indicator

When Environment Indicator is enabled, sub-menu positioning only accounts for a closed toolbar, so when a user hovers the main toolbar items for an open toolbar, the sub-menu appears positioned too far to the left, and covers the main item text labels. See below:

Gin with Environment Indicator

In addition to diverging from the default Gin behavior, this is amounts to a UI regression in that:

  1. Users who choose the open toolbar likely need/want the additional context provided by the text labels, so we shouldn't cover them, and
  2. A user who is attempting to click a main toolbar item text label will find that the label is immediately covered, and they could inadvertently click the sub-menu item that appears in it's place.

Proposed Solution

Emulate Gin's sub-menu CSS with positioning that accounts for both closed and open toolbar states.

Theming should also be done in a way that custom themes can easily override and extend the original environment indicator styles. This can be achieved through the use of CSS custom properties, as well as dedicated CSS files for themes requiring specialized styles e.g. Gin.

The existing inline CSS added via JS have been moved over into the central CSS file, as well as tested with the default Admin Toolbar as well as the various Gin Toolbar navigation modes to ensure it's not as broken.

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:

Comments

justcaldwell created an issue. See original summary.

justcaldwell’s picture

Assigned: Unassigned » justcaldwell

Working on a patch.

justcaldwell’s picture

StatusFileSize
new42.84 KB

MR15 replaces --gin-toolbar-width-collapsed with --gin-toolbar-x-offset and removes -4px/-3px margin adjustments which seem no longer necessary.

Gin actually uses --gin-toolbar-x-offset to position sub-menus. It is calculated using --gin-toolbar-width (for open toolbar) and --gin-toolbar-width-collapsed (for closed toolbar), and it also takes the screen width into account.

This change also (mostly) fixed another issue I noticed. Before the change, sub-menus had excessive margin/indentation with Environment Indicator enabled at screen widths < 976px — basically the "tablet" breakpoint (below, left). After the change, the large left margin was replaced with a small negative margin that caused sub-menus to slightly overlap the border (below, middle). To resolve that, I removed the "-4px" margin adjustment that was previously introduced by #3309626: Integration with Gin Toolbar and Admin Toolbar (below, right).

Indentation at the tablet breakpoint

This worked well at all screen widths, at least with Gin 3.0-rc1. So much so, that I also removed the (seemingly redundant?) -3px margin adjustment that was being added to <li>s by environment_indicator.js.

I still plan to test with Gin 3.0-beta5, but it seems like the margin adjustments are no longer needed. I can only assume something in Gin changed that addressed the extra margin described in #3309626: Integration with Gin Toolbar and Admin Toolbar.

I'm hoping someone else on Gin 3.0-rc1 can test these changes.

justcaldwell’s picture

Status: Active » Needs review
justcaldwell’s picture

Issue summary: View changes

After testing with Gin 3.0-beta5, here's what I've learned:

The extra margin described in #3309626: Integration with Gin Toolbar and Admin Toolbar still appears in beta5, but is no longer an issue in Gin 3.0-rc1. So, Environment Indicator no longer needs to include the negative margin adjustment for sites on Gin 3.0-rc1+. We can still support earlier versions with a fallback value:

/* TODO: Remove this when it's no longer necessary to support Gin 3.0@beta. */
@media (min-width: 61em) {
  [dir="ltr"] .gin--vertical-toolbar .toolbar-menu-administration > .toolbar-menu > .menu-item .toolbar-menu {
    margin-left: var(--gin-toolbar-x-offset, calc(var(--ginVerticalToolbarOffset) - 7px));
  }

  [dir="rtl"] .gin--vertical-toolbar .toolbar-menu-administration > .toolbar-menu > .menu-item .toolbar-menu {
    margin-right: var(--gin-toolbar-x-offset, calc(var(--ginVerticalToolbarOffset) - 7px));
  }
}

This consolidates the previous negative margin adjustments (-7px in total) in CSS, rather than having -4px in CSS, and -3px in JS. The media query prevents the margin issues at smaller breakpoints described in #4.

The margin adjustment is only applied to the fallback value (--ginVerticalToolbarOffset for beta5 and earlier), since RC1's --gin-toolbar-x-offset doesn't need it. In fact, when we use --gin-toolbar-x-offset, we're doing exactly what Gin is doing (as of RC1), so this block becomes redundant once it's no longer necessary to support older versions, and it can be removed entirely (per the TODO comment).

justcaldwell’s picture

Assigned: justcaldwell » Unassigned
devkinetic’s picture

Status: Needs review » Closed (outdated)

There is a more recent issue #3367093-8: Gin compatibility: Small screen vertical toolbar submenu items margin too big where the css has been removed along with the js to support the latest versions of Gin, which is currently at 3.0.0-rc5.

joelpittet’s picture

Status: Closed (outdated) » Reviewed & tested by the community
StatusFileSize
new133.67 KB

This still fixes an issue with gin compatibility (even without the JS) but I will re-roll.

Here is what it looks like before the patch:
before

joelpittet’s picture

Status: Reviewed & tested by the community » Needs review

I guess I changed the code so Needs Review is appropriate, I deleted those CSS lines that were marked for removal

ramil g’s picture

Status: Needs review » Reviewed & tested by the community

The patch works. Thanks! Changing to RTBC

codebymikey made their first commit to this issue’s fork.

codebymikey’s picture

Title: Gin Compatibility: Account for "open" vertical toolbar » Handle theming completely via CSS and CSS custom properties
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
Related issues: +#3487202: Gin vertical toolbar submenu's overlap, +#3482707: Gin toolbar skips background colour, but not when Core Navigation module is enabled

The original issue might already be addressed by #3487202: Gin vertical toolbar submenu's overlap as I couldn't replicate the issue on there, so updating the issue to better support for Gin (by having a dedicated CSS file) as well as improving theming support by removing most of the inline styles and JS in favour of stylesheet inheritance via CSS custom properties in the form of --environment-indicator-background and --environment-indicator-color.

trackleft2’s picture

Looks like this could be a duplicate of #3467210: Update module to use CSS variables instead of adding inline CSS via Javascript. Sorry! Would you all be willing to review the merge request on that issue, and possibly add in anything from this issue that is required to combine the two?

jds1’s picture

Following along here since I'm looking for a fix for https://www.drupal.org/project/environment_indicator/issues/3504634. I'm hopeful that this issue will be the one that fixes everything!

jds1’s picture

Status: Needs review » Needs work

PHPCS test failing on MR95. Also plain diff MR95 patch does not apply to 4.0.21

trackleft2’s picture

FYI I've added instructions to this module's .tugboat configuration file to install gin and gin_toolbar module, set gin as the default admin theme, and three environment switchers in order to help us test.

To view a demo site with a copy of the module installed with this merge request applied, click the view live preview link under this issue's summary.

Only local images are allowed.

To log in use these credentials
username: admin
password: admin

I've done the same on #3467210: Update module to use CSS variables instead of adding inline CSS via Javascript. If you'd like to review that alternate Merge request.

I've noticed that this merge request seems to break the "Default" Environment Indicator when no toolbar integration is enabled.

trackleft2’s picture

trackleft2 changed the visibility of the branch 4.x to hidden.

trackleft2’s picture

I've updated MR !95 by merging in upstream changes and rearranging things a bit in order to be more in line with where the module is headed. In order to test you must now enable the environment_indicator_toolbar module.

steveoriol’s picture

the last two patches don't work for me.
As soon as you open a submenu, it's superimposed on the parent menu, which becomes difficult to read.

trackleft2 changed the visibility of the branch rebuild-tugboat to hidden.

trackleft2’s picture

Hi @steveoriol this issue remains open because we aren't sure if it has already been resolved or not by the merged fix in #3467210: Update module to use CSS variables instead of adding inline CSS via Javascript.

There is a possibility that all issues discussed here have already been resolve in 4.1.0-alpha1.

If you discover more that needs to be done, please report back the steps needed to reproduce what you are seeing. Thank you so much for your interest in helping out.

mvonfrie’s picture

I used the MR !15 in my site to fix this issue. After updating from 4.0.21 to 4.0.25 that is not applicable anymore and it works without applying MR !129.