Problem/Motivation

Part of #3624760: Plan: consistent Display Builder entry points in the Navigation top bar. Follows #3616421: Add a navigation toolbar item in page built with Display builder, merged for its logic: route checks, cache tags, access.

#3616421: Add a navigation toolbar item in page built with Display builder renders the page layout as a featured link, "Edit page display". Featured means primary: on a node page it sits before core's "Edit", so the page gets two primary buttons, and the one with the widest reach comes first. Its label also reads as "edit this page", while it edits a layout shared by every page matching its conditions.

#3624762: Top bar entry for the main content display: bundle display or override and #3624763: Top bar entry for Views page displays add more entries to the same place, so they need an item that can hold several.

Proposed resolution

  • A text-only "Display" button in TopBarRegion::Actions, not primary, weighted after core's page_actions. Core's "Edit" stays the only featured action.
  • It opens a toolbar-dropdown listing the layers that built the current page, outer to inner, each with a second line saying how far a change reaches.
  • The item moves from display_builder_page_layout to the main module. Submodules contribute entries; this issue ships the page layout one, labelled "Page layout: label" with "Pages matching its conditions".
  • Each entry carries its own access and cacheability, merged into the item.
  • No item when no entry is accessible, so the top bar does not appear for us alone on pages with nothing to show.

Mockups: see #3624760: Plan: consistent Display Builder entry points in the Navigation top bar.

Remaining tasks

User interface changes

"Edit page display" primary button replaced by a "Display" dropdown after core's page actions.

Issue fork display_builder-3625022

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

mogtofu33 created an issue. See original summary.

mogtofu33’s picture

mogtofu33 commented
Issue summary: View changes
mogtofu33’s picture

mogtofu33 commented
Issue summary: View changes

mogtofu33 opened merge request !423

mogtofu33’s picture

mogtofu33 commented
Assigned: Unassigned » pdureau
Status: Active » Needs review
pdureau’s picture

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

Code review

Looking at the template, it seems top_bar_display is not a proper UI component because it is mixing its own markup with some from Navigation module's templates : https://git.drupalcode.org/project/display_builder/-/blob/d69bf64e4a6c8e...

<div {{ attributes.addClass(['toolbar-dropdown__menu', 'db-top-bar-display']) }}>
  <ul class="toolbar-dropdown__list">
    {% for entry in entries %}

Would it be better to simply "componentize" toolbar-dropdown in a proper SDC, without anything specific to Display Builder, we can use right now? With
So, we can also propose a Core issue to ship this template as a component and we will be able to "clean" this next year.

Feature review

Feature is great, but "Pages matching its conditions" wording is confusing in:

<a href="/admin/structure/page-layout/ccc/builder" class="toolbar-dropdown__link">
  Page layout: User profiles
  <span class="db-top-bar-display__description">Pages matching its conditions</span>
</a>

Why "Pages" in plural when rendering a single page? What is "it" in "its conditions"?

Before proposing anything, what would be the descriptions of the other buildables (entity view, overrides, view display)?

mogtofu33’s picture

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

The core issue is a good idea to help extend the navigation top bar menu, probably the 3 dots.

For the wording let's move that as a wrapping up issue, it's not blocking and as you say will make sense when everything is in place, created #3625203: Top bar wording review and adjustment added to the plan.

For our component, I think it's already generic, but ok to do something that can help a future core issue I switched to a toolbar_dropdown component to review.

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs review » Reviewed & tested by the community

The template is weird with a missing BEM block class (.toolbar-dropdown) but I guess it is to follow the existing weirdness of navigation modules templates:

 $ grep -r toolbar-dropdown web/core/modules/navigation/ | grep twig
web/core/modules/navigation/templates/top-bar-page-action.html.twig:<li class="toolbar-dropdown__item">
web/core/modules/navigation/templates/top-bar-page-action.html.twig:  {{ link|add_class('toolbar-dropdown__link') }}
web/core/modules/navigation/templates/top-bar-page-actions.html.twig:  <div class="toolbar-dropdown__menu" id={{ dropdown_id }}>
web/core/modules/navigation/templates/top-bar-page-actions.html.twig:    <ul class="toolbar-dropdown__list">

So, RTBC.

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Reviewed & tested by the community » 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 b56cb0db on 1.0.x 
    task: #3625022 Display dropdown in the Navigation top bar

By: mogtofu33...

Status: Fixed » Closed (fixed)

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