- Issue created by @lauriii
- ๐ฎ๐ณIndia anjali rathod India
const { data: pageItems, isLoading: isPageItemsLoading, error: pageItemsError, } = useGetContentListQuery('xb_page');
on PageInfo.tsx
Here we are getting the pageItems from
ApiContentControllers::list()
, which returns content values from$this->entityTypeManager->getStorage('xb_page');
entity . But since these are not published yet the value shows the last saved title or Untitled Page incase of new page. Inorder to reflect the immediate changes in the title in navigation drop down, we need to call and generate similar pageItems fromAutoSaveManager::getAllAutoSaveList()
.
This is my POV . Any feedback or suggestions are appreciated! - ๐ซ๐ฎFinland lauriii Finland
The page title should update both in the top bar in the list. This is a common pattern many tools similar to XB. If we don't show name in the list once it has been changed, it is really hard to find the right page when creating several pages in a single publish, since all of them would show up as "Untitled page".
- ๐ฎ๐ณIndia anjali rathod India
@wim leers Yes. Should I go ahead with the approach I shared in #4 ?
- Merge request !714Issue #3506854: Updated the logic to replace title of page from autosave data. โ (Merged) created by Unnamed author
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
wim leers โ credited larowlan โ .
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
๐ on the MR. @larowlan's review/suggested direction will also require an update to
openapi.yml
.P.S.: why the term ? ๐ That doesn't appear anywhere in the codebase.
- First commit to issue fork.
- First commit to issue fork.
- ๐ฌ๐งUnited Kingdom jessebaker
The page title should always reflect the autosave version of the page title.
The page title exists in 4 places -
- The value in the page data form field
- The title shown on the dropdown button in the top middle
- The title shown in the list of pages when you click the navigator
- The title shown in the list of autosaves in the review changes dropdown.
I think the CURRENT page title can only be updated from two places
- Changing the value in the form field
- Publish all changes in the rare situation that the page was renamed in another tab/by another user
Other pages' titles (shown in the navigator) can be updated in one place
- When publishing all changes
Furthermore and not mentioned thus far in this issue, the navigator also shows the pages' url alias.
So, my proposed solution
- When user types in page title, in real time, purely on the client-side we update the value in the dropdown button. This is already in place in 0.x
- When user types in page title (or any field in the form on the right) we update the preview by calling the server This is already in place in 0.x
- When user types in page title, we invalidate the cache of the pages list (
/xb/api/content/xb_page
) in the navigator (usinginvalidateTags
) - We also invalidate the cache of the pages list when user types in the URL alias,
- We also invalidate the cache of the pages list when a user "publishes all"
- On the server side, when fetching the list of pages, the page title and url alias of each page returned should be that of the autosave if there is one.
Then we must ensure that when the navigator is opened, if the cache has been invalidated, we re-fetch the list of pages.
- ๐ฌ๐งUnited Kingdom jessebaker
I've done most of the refactor that I suggested in #20
The list of pages that is returned from
/xb/api/content/xb_page
was previously updated to include a keyautoSaveTitle
. Can the same be done to also return anautoSavePath
? - ๐ฎ๐ณIndia anjali rathod India
@jessebaker by autoSavePath do you mean a path similar to
/xb/xb_page/1/editor
?
Because thepath
key value pair returned from/xb/api/content/xb_page
gives the path for the autosave entity. - ๐บ๐ธUnited States tedbow Ithaca, NY, USA
I chatted with @jessebaker about this.
This is about updating the path in the navigation to match the alias field if it was changed by the user
I am working on this
- ๐บ๐ธUnited States tedbow Ithaca, NY, USA
updated title and summary for alias updates
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
This is working around the problem introduced in #3500052, identified at #3500052-27: Provide HTTP API for listing Page content entities that can be updated by the current user โ , then discussed by @lauriii, @mglaman and I, and which @mglaman wanted to see solved in ๐ Define xb_path entity key for identifying entity path field Active . At minimum, this MR's work-around needs to point to ๐ Define xb_path entity key for identifying entity path field Active . But the consequence is that this MR will have to do some cache context trickery.
But it'd be better still to just do that first, to avoid having to introduce another layer of work-arounds here. I'm fine with being pragmatic and landing this first.
- ๐บ๐ธUnited States tedbow Ithaca, NY, USA
@wim leers re #26
But the consequence is that this MR will have to do some cache context trickery.
I don't actually need to calls
base_path()
so hopefully this issue won't have extra cache problems. see https://git.drupalcode.org/project/experience_builder/-/merge_requests/7...also I added a todo to ๐ Define xb_path entity key for identifying entity path field Active to figure out the 'path' key dynamically
- ๐บ๐ธUnited States tedbow Ithaca, NY, USA
Seems like this needs to be fixed by stable
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Great work on this one โ this looks very close! I wanted to merge it, but I couldn't help but agreeding with @larowlan's feedback WRT โจ Send entity keys to the editor from the mount controller Active : https://git.drupalcode.org/project/experience_builder/-/merge_requests/7....
Raised 2 additional code clarity concerns (nullability instead of empty string, and
autoSaveTitle
โautoSaveLabel
, which makes more sense once Lee's feedback is addressed). - ๐บ๐ธUnited States tedbow Ithaca, NY, USA
@wim leers @larowlan thanks for the reviews, will address
- ๐บ๐ธUnited States tedbow Ithaca, NY, USA
setting to Needs Review. I think I have addressed the feedback but the pipeline is failing because of 403 errors in UI and Composer jobs. I think it is related to cloudflare issues https://www.cloudflarestatus.com/incidents/gshczn1wxh74
Will try to re-run the job in a bit
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
LGTM, but the
UI eslint
CI job has been consistently failing. Probably a trivial change is needed :) - ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
RTBC, but won't be mergeable until the
UI eslint
CI job is fixed. - ๐บ๐ธUnited States tedbow Ithaca, NY, USA
Looks like @wim leers just set this back to Needs Work for the ES Lint fail which @jessebaker fixed, thanks!
- ๐บ๐ธUnited States tedbow Ithaca, NY, USA
Ok. all e2e tests passed beside media-library, reran and media-library just passed๐
-
tedbow โ
committed 50f5d277 on 0.x authored by
anjali rathod โ
Issue #3506854 by tedbow, anjali rathod, jessebaker, wim leers, lauriii...
-
tedbow โ
committed 50f5d277 on 0.x authored by
anjali rathod โ
- ๐บ๐ธUnited States tedbow Ithaca, NY, USA
2 e2e tests but re-tested and those have now passed
- Status changed to Fixed
11 days ago 3:04pm 25 April 2025 Automatically closed - issue fixed for 2 weeks with no activity.