Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-7825: UDW Blank browse tab after going from bookmarks from search #1190

Merged
merged 3 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import { getIconPath } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scri
import { getTranslator } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/context.helper';

const BookmarksTabModule = () => {
const shouldRestorePreviousStateRef = useRef(true);
const isMarkedLocationSetByBookmarksRef = useRef(false);
const restorationStateRef = useRef(null);
const restInfo = useContext(RestInfoContext);
const tabsConfig = useContext(TabsConfigContext);
const [currentView] = useContext(CurrentViewContext);
Expand All @@ -46,13 +47,24 @@ const BookmarksTabModule = () => {
};

useEffect(() => {
setMarkedLocationId(null);
dispatchLoadedLocationsAction({ type: 'CLEAR_LOCATIONS' });
const isCleared = markedLocationId === null && loadedLocationsMap?.length === 0;

if (!isCleared && !isMarkedLocationSetByBookmarksRef.current) {
restorationStateRef.current = {
markedLocationId,
loadedLocationsMap,
};

setMarkedLocationId(null);
dispatchLoadedLocationsAction({ type: 'CLEAR_LOCATIONS' });
}
}, [setMarkedLocationId, dispatchLoadedLocationsAction, markedLocationId, loadedLocationsMap]);

useEffect(() => {
return () => {
if (shouldRestorePreviousStateRef.current) {
setMarkedLocationId(markedLocationId);
Copy link
Contributor Author

@tischsoic tischsoic Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: So, how the bug worked:

  1. User searches for e.g. "a"
  2. SearchTabModule component is instantiated and stores markedLocationId and loadedLocationsMap which will be restored later on component destruction
  3. The Search component uses useSearchByQueryFetch, which clears loaded locations data on fetching search results (this is why we store these data during 2.)
  4. The user clicks Bookmark tab
  5. The SearchTabModule is destructed and functions restoring markedLocationId and loadedLocationsMap are being called but their call is not yet processed on the React level
  6. The BookmarksTabModule is being created.
  7. The BookmarksTabModule stores current markedLocationId and loadedLocationsMap in order to restore them later, but at the time of storing them, they haven't been restored yet in React as setMarkedLocationId and dispatchLoadedLocationsAction haven't been processed yet.
  8. The BookmarksTabModule performs clearance setMarkedLocationId(null); dispatchLoadedLocationsAction({ type: 'CLEAR_LOCATIONS' }); - functions are being called, but not processed yet by React.
  9. setMarkedLocationId and dispatchLoadedLocationsAction from the SearchTabModule are being processed by React but it is too late for the BookmarksTabModule to store them as it already did it.
  10. setMarkedLocationId and dispatchLoadedLocationsAction from the BookmarksTabModule are being processed by React
  11. The user clicks on the Browse tab
  12. The BookmarksTabModule is being destructed and restores wrong markedLocationId and loadedLocationsMap.

dispatchLoadedLocationsAction({ type: 'SET_LOCATIONS', data: loadedLocationsMap });
if (!isMarkedLocationSetByBookmarksRef.current) {
setMarkedLocationId(restorationStateRef.current.markedLocationId);
dispatchLoadedLocationsAction({ type: 'SET_LOCATIONS', data: restorationStateRef.current.loadedLocationsMap });
}
};
}, []);
Expand All @@ -62,8 +74,9 @@ const BookmarksTabModule = () => {
return;
}

shouldRestorePreviousStateRef.current = false;
isMarkedLocationSetByBookmarksRef.current = true;
setMarkedLocationId(bookmarkedLocationMarked);

loadAccordionData(
{
...restInfo,
Expand All @@ -90,7 +103,7 @@ const BookmarksTabModule = () => {
<div className="m-bookmarks-tab">
<Tab>
<BookmarksList itemsPerPage={tabsConfig.bookmarks.itemsPerPage} setBookmarkedLocationMarked={setBookmarkedLocationMarked} />
{renderBrowseLocations()}
{isMarkedLocationSetByBookmarksRef.current && renderBrowseLocations()}
</Tab>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useContext, useEffect } from 'react';
import React, { useContext, useEffect, useRef } from 'react';

import Tab from './components/tab/tab';
import Search from './components/search/search';
Expand All @@ -10,6 +10,7 @@ import { getIconPath } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scri

const SearchTabModule = () => {
const tabsConfig = useContext(TabsConfigContext);
const restorationStateRef = useRef(null);
const [markedLocationId, setMarkedLocationId] = useContext(MarkedLocationIdContext);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: problem analogical to the problem from Bookmarks (https://github.com/ibexa/admin-ui/pull/1190/files#r1502186952) occurs in the Search.

const [loadedLocationsMap, dispatchLoadedLocationsAction] = useContext(LoadedLocationsMapContext);

Expand All @@ -19,10 +20,23 @@ const SearchTabModule = () => {
'view-switcher': true,
};

useEffect(() => {
const isCleared = markedLocationId === null && loadedLocationsMap?.length === 0;

if (!isCleared) {
restorationStateRef.current = {
markedLocationId,
loadedLocationsMap,
};
}
}, [setMarkedLocationId, dispatchLoadedLocationsAction, markedLocationId, loadedLocationsMap]);

useEffect(() => {
return () => {
setMarkedLocationId(markedLocationId);
dispatchLoadedLocationsAction({ type: 'SET_LOCATIONS', data: loadedLocationsMap });
if (restorationStateRef.current) {
setMarkedLocationId(restorationStateRef.current.markedLocationId);
dispatchLoadedLocationsAction({ type: 'SET_LOCATIONS', data: restorationStateRef.current.loadedLocationsMap });
}
};
}, []);

Expand Down
Loading