Problem/Motivation
The instances, page layout and Views lists behave differently:
- Instance operations added by other modules (for example dev tools Import and Export) only show on the instances list.
- No list tells whether a display is published.
- Columns differ: Last log on two lists, no Updated date on page layouts, and the Views list shows the profile ID instead of its label.
- The Views list is not sorted, its pager never works, and it offers no Edit operation.
- The Instances panel misrepresents empty displays and unpublished drafts.
Each list page also carries its own description, rendered in the page body instead of Drupal's help region, and the documentation assumes Standard creates the Article content type, which it no longer does since Drupal 11.4 (#3159848: [Policy] Always install Drupal with Standard on the UI, pared down of use case specific elements (content types, node listing, commenting, theme)).
Proposed resolution
Lists
- The page layout and Views lists call
hook_entity_operation_alter()with the row's instance, so instance operations show on every list. - A Publication column on all three lists: Empty, Not published, Unpublished changes or Published. Drafts and published sources are compared without their node IDs.
- Status badges use
default_admin'sgin-statusclasses, plusdb-*classes. - Same columns everywhere: Profile label, Publication, Updated. Last log is removed.
- The Views list is sorted by Updated, newest first, with 50 rows per page. The page layout list stays unsorted and unpaged: its order is the matching priority.
- The Views list offers the operations of an entity list, in weight order: Build, Edit (the view display), Change profile (the display's Display Builder option form) and Delete.
- The Instances panel shows no indicator for an empty display, and "Not published yet" for an unpublished draft.
- New helpers in
DisplayBuilderHelpers:hasUnpublishedChanges(),publicationBadge(),statusBadge().
Note: Claro fallback is ok, only badge colors (published / empty...) are not in Claro, which for me is ok as it's a deprecated admin theme. Gin 5 is good as it rely on the same classes as default_admin.
Help and documentation
- Page descriptions move to
hook_help(), so they render in Drupal's help region: instances, profiles, pattern presets, page layouts and the Views list, plus the profile add, edit and panel settings forms and the page layout add, edit and duplicate forms. - Each help text links to its own documentation page instead of the documentation home.
- New tutorial, Build a homepage: a content type, its display, the front page setting, and a page layout with the
<front>condition, from a Drupal install without the recipes. - Build your first display explains that Standard no longer creates Article, and adds it with
drush recipe core/recipes/article_content_type.

Remaining tasks
- Follow-up:
Instance::isPublishedPresent()compares raw hashes, node IDs included, while the lists compare content without node IDs, so the builder and the lists can disagree:- On an empty, never published display, it returns FALSE: Publish is enabled with nothing to publish, and Save status shows "Saved".
- On a published display whose published sources carry fewer node IDs than the draft, it returns FALSE although nothing changed: Publish is enabled, Restore is offered, and Save status shows "Saved" instead of "Published".
Using
DisplayBuilderHelpers::hasUnpublishedChanges()there would align both. - Follow-up: #3623275: Replace the instances admin list with a View
User interface changes
New Publication column, same columns on all lists, a sortable and paged Views list with Edit and Change profile operations, and page help in Drupal's help region.
API changes
New static helpers in DisplayBuilderHelpers. ViewsManagementController takes two more autowired constructor arguments.
Data model changes
None.
| Comment | File | Size | Author |
|---|
Issue fork display_builder-3623326
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:
- 3623326-pdu
changes, plain diff MR !415
- 3623326-consistent-display-lists
changes, plain diff MR !403
Comments
Comment #3
mogtofu33 commentedComment #4
mogtofu33 commentedSorry, set too soon in review, I have small fixes to add.
Comment #5
mogtofu33 commentedComment #6
mogtofu33 commentedComment #7
mogtofu33 commentedComment #8
pdureau commentedFeature review
Ok, and it looks better 👍
OK 👍
Checked:
So, the features changes are OK.
Code review
5 public static methods not clearly related and not covered by any interfaces have been added to
DisplayBuilderHelpersclass.This may seem handy at first but this will add more work to do later: #3606952: Remove DisplayBuilderHelpers
So can we try to avoid this?
::instanceCells(): why factoring such a simple data structure (2 properties, used only 3 times)?::statusBadge(): why not a render element or a SDC component?::publicationBadge(): used only by::instanceCells(), can be private or moved there::hasUnpublishedChanges(): belong to Instance entity?::contentHash(): belong to Instance entity?Documentation update
About the new docs/build-a-homepage.md, do we want to show how to do a "landing page" or the "homepage"?
Following our discussion of last weekly, where we talked about the Canvas' Page Content Entity Type, I guess it is the former.
By focusing on the landing page use case, we can get a smaller and friendlier documentation, with:
Remaining tasks
It may explain some weird published status I got last weekend 🤔
Comment #9
mogtofu33 commentedThanks for the review.
DisplayBuilderHelpers: the 5 new methods are gone, the class is back to its 1.0.x state, in line with #3606952: Remove DisplayBuilderHelpers.
::contentHash()is nowInstance::getContentHash(), next to::getUniqId()onInstanceInterface.::hasUnpublishedChanges()moved toInstance. It stays static: the Instances panel compares hashes it batch-loads, without loading each buildable.::publicationBadge()is replaced byInstance::getPublicationStatus(), returning aPublicationStatusenum (Empty, Not published, Unpublished changes, Published).::statusBadge()is now adisplay_builder:status_badgeSDC component.::instanceCells()is inlined in its 3 callers.Documentation: I kept the homepage angle but told it through a landing page, so a search on either finds it. The tutorial is now "Build a homepage from a landing page": it creates a Landing page content type, and steps 1 to 4 alone build any landing page. The front page and page layout steps follow for those who want a homepage.
isPublishedPresent(): left for a follow-up, it compares hashes that include node IDs, so it can report unpublished changes where there are none.
Comment #10
pdureau commentedI feel we may have reached some limits of "AI-acceleration" here 🤗
DisplayBuilderHelpers:
Moving the methods to Instance class are not only a cut-paste operation, we can add our little human touch here.
Why does it need to stay static now we are in the Instance entity and we always have an Instance entity instantiated each time we use it? The data is already here, let's use it.
I may be wrong, so I will try something in a separated branch. So, kept assigned to me.
I will need to have a deeper look.
Documentation
Better but... something is still off. Maybe because it is written by an LLM. But let's move on and address this in a ticket about documentation in general.
Comment #12
mogtofu33 commentedFair point, the static methods on
InstanceInterfaceare a leftover of the helpers' shape, not a need.One constraint to keep in mind for your branch: the Instances panel already loads the instances with
::loadMultiple(), but it takes the published hash fromDisplayReference, collected by each buildable while listing. That avoids loading every buildable, and its config or entity, a second time per row. So an instance method would still need a way to receive that hash from outside, or the panel pays one buildable load per row.Created a wider approach and ctritics for our public contracts to ease contrib #3624396: Plan: Harden InstanceInterface, IslandInterface and DisplayBuildableInterface as public API before RC1, so some work here will be impacted and even could become not relevant, like deprecation of
getUniqId()And we have hash changes from discovery in #3555110: Symmetric translation, with specific bugs emerging, IMHO would be more efficient to handle that as child of #3624396: Plan: Harden InstanceInterface, IslandInterface and DisplayBuildableInterface as public API before RC1 instead of here.
Comment #13
pdureau commentedThanks for the alert. So, i will not play risky games with
::contentHash()and stop here.So, only one additional commit in the 415 MR:
Ready for review.
Comment #14
mogtofu33 commentedOups, edited while reply, so my edit highlight on my previous comment was to create child of #3624396: Plan: Harden InstanceInterface, IslandInterface and DisplayBuildableInterface as public API before RC1 after this and translation merge, in relation with #3606952: Remove DisplayBuilderHelpers and #3621008: Move DisplayBuildable form logic to a dedicated class to have more dedicated tasks if ok.
Comment #15
mogtofu33 commentedSo remaining contract points can go child of #3624396: Plan: Harden InstanceInterface, IslandInterface and DisplayBuildableInterface as public API before RC1:
::getContentHash()off the interface,::isPublishedPresent()and::hasUnpublishedChanges()giving one answer, which hash to use when, and whetherPublicationStatusis public API. Nothing left to block this MR on that side.Comment #16
mogtofu33 commented