Problem/Motivation

The 3px top border [1] added by environment_indicator causes a wrong top-value [2] for the .gin-secondary-toolbar--frontend, so the first 3 pixels are cropped.

Steps to reproduce

Proposed resolution

Adding a 3px margin top to the .gin-secondary-toolbar--frontend seems to be a proper fix.
Changing the top-value won't work, because the bar has position:sticky, so the top-value only takes effect, when the bar gets stuck.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#12 gin_settings.png49.97 KBthomas.frobieter
#12 enviroment_indicator_gin_second_toolbar.png37.77 KBthomas.frobieter

Issue fork environment_indicator-3388814

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

thomas.frobieter created an issue. See original summary.

thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
commented

For any reasons, the 3px default border is overridden by inline style.
So if this is configurable by the user, this issue is not fixable without moving this value into a CSS variable.

Edit: Okay, the 6px inline style is set by environment_indicator.js. It doesn't make any sense to me, that these values differ. Both should be 3px or 6px.

thomas.frobieter opened merge request !30

thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
commented
Assigned: thomas.frobieter » Unassigned
Status: Active » Needs review

Please review:

  1. Added the border-width as CSS variable to have this as global "truth"
  2. Replaced all static values in the CSS with the new variable
  3. Grab the CSS variables value in the Javascript file and also replaced it with the static values
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Reviewed & tested by the community

Whao this is really cool and was hard to find, I was wondering, why the margins were different! Nice catch!
Using a CSS variable totally makes sense here.

RTBC from my side, I can't see anything that could be done better, but let's hear what the maintainers think.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Just one comment left for maintainer discussion, otherwise RTBC.

thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
commented
Status: Reviewed & tested by the community » Needs review

Added a fallback value!

grevil’s picture

grevil commented
Status: Needs review » Reviewed & tested by the community

LGTM!

thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
commented
Status: Reviewed & tested by the community » Fixed
thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
commented
Status: Fixed » Reviewed & tested by the community
isholgueras’s picture

isholgueras commented

Hi!

I've been reviewing this MR and I can't reproduce or identify the issue under Drupal 10.1.x, environment_indicator 8.x-4.x, gin 8.x-3.0-rc6 and gin_toolbar 8.x-1.0-rc3.

I'm changing between environment indicator 4.x branch and 3388814-fix-gin-admin branch, after clearing the cache, and I can't find any difference.

For Gin settings, I've tested Sidebar, Vertical Toolbar, Horizontal, Modern Toolbar and Legacy and also the Secondary Toolbar in the Frontend.
For the Environment indicator, I've tested the Toolbar integration enabled and disabled.

Can someone post some screenshots or describe a bit more the issue and how to reproduce it?

Another issue that I can find in the MR is that in this line: (https://git.drupalcode.org/project/environment_indicator/-/merge_request...), there is the use of the CSS variable --border-width, but this variable is not defined.

Thanks!

thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
commented
Issue summary: View changes
StatusFileSize
new enviroment_indicator_gin_second_toolbar.png37.77 KB
new gin_settings.png49.97 KB

Hi @isholgueras, no problem, only the Modern Toolbar and probably the Horizontal Toolbar are affected by this issue, because the vertical toolbar doesn't have the second bar next to it.

Heres a screenshot:

The Gin configuration looks like this (Gin Toolbar Module enabled):

Another issue that I can find in the MR is that in this line: (https://git.drupalcode.org/project/environment_indicator/-/merge_request...), there is the use of the CSS variable --border-width, but this variable is not defined.

Please see: https://git.drupalcode.org/project/environment_indicator/-/merge_requests/30/diffs#e05f17ede7c46d4798c5f60472b1cfc257aeee3a_1_1

thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
commented

Another issue that I can find in the MR is that in this line: (https://git.drupalcode.org/project/environment_indicator/-/merge_request...), there is the use of the CSS variable --border-width, but this variable is not defined.

Oh sorry, its about --border-width in the js file, not about --enviroment-indicator-border-width. I'll check this.

thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
commented

Good hint, fixed. I missed that when I replaced the original variable name, thx!

isholgueras’s picture

isholgueras commented
Status: Reviewed & tested by the community » Fixed

Thanks for explaining it. Yes, now I can reproduce the issue and this patch fixes the issue.
Thanks!

isholgueras’s picture

isholgueras commented
Status: Fixed » Closed (fixed)