Problem/Motivation

We are targeting Drupal 11.3 minimum where:

  • Gin 6 will become the admin theme
  • Navigation will be out of experimental and (I guess) activated by default

In the same time, this is issue is stalled because of the incompatibility between Navigation and Admin toolbar #3529095: Adopt navigation module's top bar

However, there is something "safe" we can take from this other issue: the cleaner look.

Proposed resolution

Tasks:

Bug fixes:

  • Toolbar button alignment
  • Form details icon position
  • Shoelace menu items are HTML checkboxes. Some themes (like taiwlind) are styling checkboxes a bit aggressively.
  • Drawer title overflow
  • Hide the weird form API artefact

Out of scope:

  • change z-index, absolute positioning. Jean did a lot effort to make them compatible with toolbar, admin toolbar, navigation...
  • Other branding compliance: colors, icons, button shape... let's do it after beta1, taking the time
CommentFileSizeAuthor
#15 display_builder_theme_test.png20.54 KBpdureau
#12 olivero.png19.36 KBpdureau
#12 mercury.png19.98 KBpdureau
#12 dsfr.png21.16 KBpdureau
#12 daisyui.png21.87 KBpdureau
#12 bootstrap.png22.66 KBpdureau

Issue fork display_builder-3555193

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

pdureau created an issue. See original summary.

pdureau’s picture

pdureau commented
Issue summary: View changes
pdureau’s picture

pdureau commented
Status: Active » Needs work

Work has started

pdureau’s picture

pdureau commented
Issue summary: View changes

Out of scope: change z-index, absolute positioning. Jean did a lot effort to make them compatible with toolbar, admin toolbar, navigation...

Compatibility with navigation can be improved without hurting compatibility with toolbar and admin_toolbar by fixing --drupal-displace-offset-top fallback value

pdureau opened merge request !152

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Need review.

Tested with the toolbar, admin toolbar & navigation.

The removal of #3529095: Adopt navigation module's top bar logic can create a weird layout on smaller screen, but I am proposing to address this in #3544782: More responsive toolbar (see new proposal).

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

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Needs review » Postponed (maintainer needs more info)
pdureau’s picture

pdureau commented
Assigned: Unassigned » pdureau
Status: Postponed (maintainer needs more info) » Needs work

I don't agree with the global increase of margin / padding

OK. The most important is consistency, so i will propose smaller margin-x in .db-toolbar

Element Viewport Current value Proposed value
.db-toolbar__start, .db-toolbar__end default --sl-spacing-medium (16px) --sl-spacing-small (12px)
.db-toolbar sl-tab::part(base) default --admin-toolbar-space-16 (16px) --admin-toolbar-space-12 (12px)
.db-toolbar sl-tab::part(base) max-width: 1280px --admin-toolbar-space-12 (12px) --admin-toolbar-space-8 (8px)
.db-toolbar__end > *:not(:first-child, dialog, :empty) default --admin-toolbar-space-8 (8px) --admin-toolbar-space-4 (4px)
.db-toolbar__end > *:not(:first-child, dialog, :empty) max-width: 1280px --admin-toolbar-space-4 (4px) --admin-toolbar-space-4 (4px)
pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Proposal pushed to MR

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Needs review » Needs work

We have a problem with checkboxes and DSFR but it's a problem to fix in the theme.

The Playwright test failing highlight that the sidebar are now behind the toolbar and cannot be closed in fullscreen.

See artifact of the job: test-failed-1.png

pdureau’s picture

pdureau commented
StatusFileSize
new bootstrap.png22.66 KB
new daisyui.png21.87 KB
new dsfr.png21.16 KB
new mercury.png19.98 KB
new olivero.png19.36 KB

The Playwright test failing highlight that the sidebar are now behind the toolbar and cannot be closed in fullscreen.

See artifact of the job: test-failed-1.png

Indeed, I see that in the screenshot, but I ma not reproducing (with or without admin toolbar activated):

Bootstrap:
a
Daisy:
a
Olivero:
a
DSFR:
a
Mercury:
a

Investigation ongoing.

pdureau’s picture

pdureau commented
mogtofu33’s picture

mogtofu33 commented

I can reproduce locally, with a php server running, from the module launch :
npx playwright test page --project=firefox --headed # To view the test
npx playwright test page --project=firefox --ui # To follow the test

You will see the test being stuck by not being able to close.

The test use the theme test which is based on claro, perhaps a missing css variable.

pdureau’s picture

pdureau commented
StatusFileSize
new display_builder_theme_test.png20.54 KB

I am still not reproducing while using display_builder_theme_test with Firefox outside of Playwright:
display_builder_theme_test

Investigation still ongoing.

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

All green :)

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

hello :)
do we merge guys?

pdureau’s picture

pdureau commented

No need to worry too much about form rendering in the contextual panels because it will be solved by #3535999: Render UI with both front and admin theme

pdureau’s picture

pdureau commented

do we merge guys?

if you tested and it is OK for you, put it in RTBC :) It will make Jean's review easier.

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • mogtofu33 committed 870d7146 on 1.0.x authored by pdureau 
    chore: #3555193 Navigation look without navigation module

By: pdureau

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.