Overview
Experience Builder provides a working space where content creators can work on several changes at once. In order to provide a seamless editing experience, it is desired to make navigating between pages as seamless as possible by implementing navigation within Experience Builder. For this reason, we have implemented a navigator which is initially used for just XB pages. This will be expanded in future to include content templates and later even CMS content. Eventually, users will be able to navigate between different content creations UIs without having to leave Experience Builder.
Proposed resolution
The navigator already has a text field for searching for content in the editor navigation that is not functional. Update `\Drupal\experience_builder\Controller\ApiContentControllers::list` to read a `search` query parameter and do a `LIKE` query on the `label` entity key.
Update `contentApi` and `getContentList` to pass the `search` query param.
Update `PageInfo` to pass a function to `Navgation` component's `onSearch` prop to perform a refresh of `useGetContentListQuery` when search is performed.

User interface changes
Pressing "enter" in search no longer shows the "not implemented yet" alert and pages can be searched
| Comment | File | Size | Author |
|---|---|---|---|
| #41 | 3518292-refactored-for-maintainability.diff | 15.12 KB | wim leers |
Issue fork experience_builder-3518292
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:
Comments
Comment #2
wim leersIf only we had
QUERY!But at least in this case, the querystring itself should be comparatively tiny, so
?search=hello%20worldwill do just fine 👍Note:
ApiContentControllers::list()may end up finding hundreds of matches. But the API response does not yet support pagination. Thoughts?Comment #3
mglamanTechnically it'd still be a subset of the default result without a query string. But I do think getting pagination is going to become important pretty soon
Comment #5
deepakkm commentedComment #7
deepakkm commentedAll the threads are resolved hence moving into review.
Comment #9
wim leersComment #10
deepakkm commentedComment #11
wim leersComment #12
deepakkm commentedComment #13
wim leersPer @penyaskito's review.
Comment #14
catchI'm a bit confused why this is a 1.0 blocker, there's already ways to search for content in Drupal. Where are the requirements for this feature documented?
Comment #15
lauriiiAdded some more context to the issue summary.
Comment #16
deepakkm commentedI think its good to go now. Hence moving it in review.
Comment #17
wim leers@catch's question in #14 is totally fair. Can we please get the designs we're implementing linked here, @lauriii? 🙏
See detailed review on the MR. Test coverage is not complete enough yet for this to be mergeable. 😅
Comment #18
lauriiiLinking designs from #3500054: Create a navigation modal for changing pages in the editor.
Comment #19
deepakkm commentedcreated a separate ticket to cover autocomplete feature as per discussion here https://www.drupal.org/project/experience_builder/issues/3522488
Comment #20
deepakkm commentedComment #21
deepakkm commentedComment #22
deepakkm commentedComment #23
wim leers#19: @deepakkm: Please see my response at #3522488: Follow-up for #3518292: `ApiContentControllers::list()`: search should exclude entity query matches for entities with auto-save data, can you please clarify it?
Comment #24
deepakkm commentedComment #25
deepakkm commentedComment #26
wim leersThe
List with searchtest case ofApiContentControllersListhas been failing.Also: I found a pretty massive bug which indicates the current test coverage is not adequate.
Comment #27
deepakkm commentedComment #28
wim leersI know the sense of urgency y'all were under, so I spent ~30 mins reviewing this during my PTO today.
This MR is unfortunately not at all ready.
navigation.cy.jsis consistently failing on the new test coverage, even after I re-tested it.… and that's just the big ones. There's plenty of smaller concerns, most of which I pointed out long ago. I have not yet re-reviewed all closed MR threads, because a bunch seem to have been closed prematurely 🙈
Comment #29
deepakkm commentedComment #30
deepakkm commentedComment #31
lauriiiComment #32
deepakkm commentedComment #33
deepakkm commentedMostly covered everything, but in case something is left do let me know.
Comment #34
deepakkm commentedComment #35
wim leersDid a high-level review. Addressed all my own nits. Since this touches auto-save things I'd like that to be reviewed by @tedbow or @larowlan.
And I'd definitely like @larowlan to do a thorough review of the changes to
ApiContentControllersComment #36
larowlanAddressed the test fails and remaining items I could.
There are 3 items left that I'm not sure about but @deepakkm is best placed to answer
1 of them I think is already handled in a follow-up
The other two I'm not sure of the status/reasoning
Comment #37
deepakkm commentedThis is good for a review now.
Comment #38
deepakkm commentedComment #41
wim leersI wanted to review, RTBC and merge this, but found myself unable to — so I spent a few hours getting this ready for commit.
Searching article nodes (which have XB fields and are editable in XB) did not work yet. And that's apparently intentional; but the route definition does not make that clear, nor does the bounds checking. Hardened that, to pave a clearer path for #3498525.
Most importantly: the logic in
\Drupal\experience_builder\Controller\ApiContentControllers::list()would've made XB harder to maintain, because its complexity was disproportionate to the functionality it provided: it was very complicated, not helped by the 8 (!!!) helper methods, incorrectly named variables and unfortunately about a dozen small bugs that made it even harder to understand. 🫣Before vs after: improved maintainability and quite literally a simpler MR:
(See attached diff. @deepakkm, I hope you find the sequence of commits I pushed to this MR useful as a reference in the future. I tried many times to express this in MR comments, but I seem to have failed to convey my message. This is probably the only MR with >200 comment threads. Given this MR needs to land before #3515932: Milestone 1.0.0-beta1: Enable creation of non-throwaway sites, and the GitLab UI is starting to struggle to still load at all, I figured it's reasonable for me to step in and show by example what I failed to convey in my review comments.)
Landed this despite one known bug: #3533096: Follow-up for #3518292: fix front-end performance and scalability of search functionality in navigator
Also ran into a pre-existing bug: #3529924-17: Add access check for using Experience Builder at all: if >=1 content entity type with an XB field can be created or edited..
Comment #43
wim leersAlso, as described at #3522488-3: Follow-up for #3518292: `ApiContentControllers::list()`: search should exclude entity query matches for entities with auto-save data, I don't understand what that issue is about, so I requested failing tests to illustrate the problem @deepakkm spotted 🙏
That means there's now 2 follow-ups for this that are now unblocked:
🚀
Glad to have this in, it works great! 🤩