- Issue created by @jabeler
- First commit to issue fork.
- ๐จ๐ญSwitzerland saschaeggi Zurich
That's a good idea, we should at least add a 1px separation ๐
- First commit to issue fork.
- Status changed to Needs review
about 1 year ago 12:27pm 18 December 2023 - ๐ฎ๐ณIndia Meeni_Dhobale
I added some spacing between top-level menus.
Personally, I would go with a bit more space.
I think 6px would be closer to what the original Gin sidebar spacing is.
Here's a few screenshots comparing 4px, 6px, and 8px.
- ๐ฎ๐ณIndia Anjali Mehta
Anjali Mehta โ made their first commit to this issueโs fork.
I am unable to apply MR getting this error -
Checking patch dist/css/layout/navigation.css... error: while searching for: margin-inline: 0; } .gin--navigation .toolbar-menu__item--level-1 > .toolbar-link:focus { outline: none; box-shadow: 0 0 0 1px var(--gin-color-focus-border), 0 0 0 4px var(--gin-color-focus); error: patch failed: dist/css/layout/navigation.css:950 error: dist/css/layout/navigation.css: patch does not apply Checking patch styles/layout/navigation.scss... Hunk #1 succeeded at 122 (offset -5 lines). Checking patch dist/css/layout/navigation.css... error: while searching for: } .gin--navigation .toolbar-menu__item--level-1 { margin-bottom: 2px; } .gin--navigation .toolbar-menu__item--level-1 > .toolbar-link:focus { error: patch failed: dist/css/layout/navigation.css:951 error: dist/css/layout/navigation.css: patch does not apply Checking patch styles/layout/navigation.scss... Hunk #1 succeeded at 123 (offset -5 lines).
- Status changed to RTBC
12 months ago 10:44am 11 January 2024 - ๐ฎ๐ณIndia divya.sejekan
Verified the Fix with MR '3409305-add-spacing-between'
Additional Spacing of 6px is added between the top menus , and looks goodTesting steps :
1. Enable Gin theme
2. Navigate to setting and enable experimental new menu , Save
3. Verify the admin menu tool barMoving To RTBC++
- Status changed to Needs work
12 months ago 11:00am 11 January 2024 - ๐จ๐ญSwitzerland saschaeggi Zurich
6px is way to wide and should be reduced to 2px max. Also we should leverage our CSS variables here instead of fixed values.
Moving back to needs work
- First commit to issue fork.
- Status changed to Needs review
12 months ago 1:00pm 11 January 2024 - ๐ฎ๐ณIndia viren18febS
6px is way to wide and should be reduced to 2px max. Also we should leverage our CSS variables here instead of fixed values.
Please review.
- First commit to issue fork.
- ๐ฎ๐ณIndia djsagar
Font variables should be created in legacy.scss
For example:
// Spacings --ginSpacingTiny: var(--gin-spacing-xxs); --ginSpacingXsmall: var(--gin-spacing-xs); --ginSpacingSmall: var(--gin-spacing-s); --ginSpacingDefault: var(--gin-spacing-m); --ginSpacingMedium: var(--gin-spacing-l); --ginSpacingLarge: var(--gin-spacing-xl); --ginSpacingXlarge: var(--gin-spacing-xxl); --ginSpacingXxlarge: var(--gin-spacing-xxxl);
- Status changed to Needs work
12 months ago 2:21pm 11 January 2024 - First commit to issue fork.
- Status changed to Needs review
12 months ago 7:01am 12 January 2024 - ๐ฎ๐ณIndia djsagar
Applied MR #17 and reviewed changes as per the comment #11
6px is way to wide and should be reduced to 2px max. Also we should leverage our CSS variables here instead of fixed values.
it's looks fine, now spacing between items for experimental navigation is 2px.
For reference:-
After MR
RTBC ++
Wait for maintainer review.
Thanks!
- Status changed to RTBC
12 months ago 8:32am 12 January 2024 - Status changed to Fixed
12 months ago 8:44am 12 January 2024 Automatically closed - issue fixed for 2 weeks with no activity.