Problem/Motivation

When we are in the display builder instances we don't have an easy way to leave and we need to rely on browser bac k button or the admin toolbar

Proposed resolution

Add a method in WithDisplayBuilderInterface and implement it 4 times:

  • in Page Layout: returns entity.page_layout.edit_form (/admin/structure/page-layout/{page_layout})
  • in Entity View display: return an URL object built from entity.entity_view_display.{entity_type}.view_mode
  • in Views: return an URL object built from entity.view.edit_display_form
  • in Devel: return an URL object built from display_builder_devel.edit

The name of this method needs to be discussed.

Use this method

  • in Display Builder's devel /admin/structure/display-builder/index in a new table col, for quick access to the config where the display builder was initialized
  • in a new "button" island plugin to put in the default config, alongside "state". The name and the wording of this island must be discussed.

Issue fork display_builder-3538607

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.

mogtofu33’s picture

mogtofu33 commented
Assigned: Unassigned » mogtofu33
mogtofu33’s picture

mogtofu33 commented

In the meantime we could clean the lists part for instances which is a bit everywhere, we have multiple path, class names and opertations, let's try to have a main DisplayBuilderListBuilder class with getOperationLinks, and each module extends if needed.
Route path, names and page titles must be consistent as well.

I don't have option on titles, just proposition to make it consistent.

Page

  • List of page config with display config with instances
  • /admin/structure/page-layout
    • @todo move to /admin/structure/page-layout/display-builder for consistency
  • title: Page layouts
    • @todo change to: Page layouts with Display Builder
  • PageLayoutListBuilder
    • entity::getDefaultOperations + build display
    • @todo change to entity::getDefaultOperations + DisplayBuilderDevelHelper::getOperationLinks

Views

  • List of instances in Views
  • /admin/structure/views/display_builder
    • @todo move to /admin/structure/views/display-builder for consistency
  • title: Display builders in Views
    • @todo change to: Views with Display Builder
  • ViewsManagementController::pageViewsIndex => @todo rename to ViewsDisplayBuilderList
    • has delete custom operation + build display
    • @todo change to delete custom operation + DisplayBuilderDevelHelper::getOperationLinks

Entity - @todo nothing yet

  • List of instances in Entities
  • @todo create /admin/structure/entity/display-builder
  • title : Entities with Display Builder
  • EntityDisplayBuilderList
    • DisplayBuilderDevelHelper::getOperationLinks

UI

  • Nothing yet, move from Devel
  • @todo: list of instances moved from devel
    • /admin/structure/display-builder/index
    • title: Entities with Display Builder
    • DisplayBuilderList
      • DisplayBuilderDevelHelper::getOperationLinks

Devel @todo move to UI

  • List of all instances
  • /admin/structure/display-builder/index
  • title: Display Builder all instances
  • DisplayBuilderDevelController
    • DisplayBuilderDevelHelper::getOperationLinks
pdureau’s picture

pdureau commented

Great idea. Standardization is welcomed. Maybe we can do it in a dedicated issue?

My personal feedbacks:

Page

  • List of page config with display config with instances
  • /admin/structure/page-layout
    • @todo move to /admin/structure/page-layout/display-builder for consistency >> ⚠️ So what would be the content of /admin/structure/page-layout/ ?
  • title: Page layouts
    • @todo change to: Page layouts with Display Builder >> ⚠️ OK, that's good because explicit, but let's keep the menu entry in /admin/structure simply "Page layouts" ?
  • PageLayoutListBuilder
    • entity::getDefaultOperations + build display
    • @todo change to entity::getDefaultOperations + DisplayBuilderDevelHelper::getOperationLinks

Views

  • List of instances in Views
  • /admin/structure/views/display_builder
    • @todo move to /admin/structure/views/display-builder for consistency >>👍 cool
  • title: Display builders in Views
    • @todo change to: Views with Display Builder >> 👍 way better
  • ViewsManagementController::pageViewsIndex => @todo rename to ViewsDisplayBuilderList >> 👍 nice
    • has delete custom operation + build display
    • @todo change to delete custom operation + DisplayBuilderDevelHelper::getOperationLinks

Entity - @todo nothing yet

  • List of instances in Entities
  • @todo create /admin/structure/entity/display-builder >> ⚠️ /admin/structure/entity is not a thing. Can we put it in an existing position in admin hierarchy? I don't see one to be honest. That may be the hint this list is not expected. What would be the purpose of this list (maybe too overkill for site builders and developers already have /admin/structure/display-builder/index)?
  • title : Entities with Display Builder >> ⚠️ Would "Entity displays with Display Builder" be better? Or maybe "Entity view displays with Display Builder" to avoid future confusion with display_builder_entity_form?
  • EntityDisplayBuilderList
    • DisplayBuilderDevelHelper::getOperationLinks

UI & Devel

⚠️ are you sure it would not be confusing for users to have access to /admin/structure/display-builder/index ? It looks like low level information and I am afraid we will move it back to Devel after some users feedbacks in beta phase.

  • mogtofu33 committed 569ed6ec on 1.0.x 
    Issue #3538607 by pdureau, mogtofu33: Pre task: consistent paths dash
pdureau’s picture

pdureau commented
Issue tags: -display_builder-1.0.0-alpha4 +display_builder-1.0.0-beta
mogtofu33’s picture

mogtofu33 commented
pdureau’s picture

pdureau commented
Issue tags: -display_builder-1.0.0-beta +display_builder-1.0.0-alpha5

mogtofu33 opened merge request !90

mogtofu33 changed the visibility of the branch 3538607-ui-refactor to hidden.

mogtofu33 opened merge request !104

mogtofu33’s picture

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

pdureau commented
Assigned: Unassigned » pdureau
pdureau’s picture

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

Test feature:

  • Page Layout : ✅, works as expected
  • View: ✅, works as expected
  • Entity View: ❌: Symfony\Component\Routing\Exception\RouteNotFoundException: Route "entity.entity_view_display.node.teaser" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 214 of core/lib/Drupal/Core/Routing/RouteProvider.php).
  • Entity View Override: ✅, no button because there is already the tabs

To discuss in a follow-up issue for beta1 (because changing the pluginId will alter config storage):

  • Wording: "Parent link" & "Allow a direct link in parent display." >> Naming things is hard. I don't have a better proposal right now but I guess we can find one
  • Icon in button: we keep the "upload" one ?
  • Add title attribute to button.
mogtofu33’s picture

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

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

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

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

Maintainers, please credit people who helped resolve this issue.

pdureau’s picture

pdureau commented
Status: Fixed » Closed (fixed)

mogtofu33 closed merge request !90