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-7901: [UDW] Removing from bookmarks content is still visible on bookmark list #1209

Merged
merged 3 commits into from
Mar 22, 2024

Conversation

GrabowskiM
Copy link
Contributor

@GrabowskiM GrabowskiM commented Mar 13, 2024

Question Answer
JIRA issue https://issues.ibexa.co/browse/IBX-7901
Type bug
Target version v4.6
BC breaks no
Doc needed no

Bookmarks are kept in other place than LoadedLocationsMapContext (which is modified after clicking "Remove from bookmarks", so bookmarks state has to be reload as well.

Bookmark children (BookmarksList and Browse) are rendered conditionally now on existing restorationStateRef.current.
This ref is set on first call of useEffect when markedLocationId is cleared to be ready to work with bookmarks module.
It serves as guardian - without it there's a moment, when markedLocationId has some garbage data from previous screen and this data is used inside bookmarks (in my case - in bookmarks list) and shows bugged screens therefore.

How it worked:

  1. BookmarksTabModule (BTM) was initialized, there was some previous data in markedLocationId
  2. first render of BTM occured, its useEffect was called (see no5)
  3. children of BTM were initialized (BookmarksList (BL) to be specific)
  4. first render of BL occured, its useEffect was called. Both were using markedLocationId with previous data, because of it sth unexpected happened
  5. useEffect of BTM (from no2) has executed, it properly cleared markedLocationId
  6. BTM was rerendered after state changed (from no5), most notably BL was rerendered and called its useEffect, as markedLocationId has been changed - now everything SHOULD BE ok
  7. But it's not, because as markedLocationId changed according to BL component, it loaded wrong data (it showed bookmarks twice in specific scenario)

How it should work (and works after these changes):

  1. BookmarksTabModule (BTM) was initialized, there was some previous data in markedLocationId
  2. first render of BTM occured, its useEffect was called (see no5)
  3. children of NTM AREN'T initialized, as they show based on restorationStateRef which is set inside useEffect (from no2)
  4. useEffect of BTM (from no2) has executed, it properly cleared markedLocationId and set restorationStateRef
  5. BTM was rerendered after state changed (from no4), as now restorationStateRef exists, BTM renders its children as well
  6. BookmarksList (BL) was initialized, it was rendered at its useEffect was called, this time with empty markedLocationId
  7. Now everything is ok, as there was no state change and useEffect was called only once, there was no duplicating bookmarks this time

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (master for features, the oldest supported for bugs).
  • Asked for a review (ping @ibexa/engineering).

@GrabowskiM GrabowskiM changed the base branch from main to 4.6 March 15, 2024 16:40
@barbaragr barbaragr self-assigned this Mar 21, 2024
Copy link

sonarcloud bot commented Mar 21, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@dew326 dew326 merged commit 79ce1a1 into 4.6 Mar 22, 2024
22 checks passed
@dew326 dew326 deleted the IBX-7901 branch March 22, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants