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

UIIN-3137: Add the isSearchToggleHitInBrowse flag to the history state to use in the condition that specifies whether to use the default sort on mount. #2682

Merged
merged 4 commits into from
Dec 2, 2024

Conversation

Dmytro-Melnyshyn
Copy link
Contributor

Purpose

Use previously applied sorting after switching from Browse search by pressing the Search toggle.

Approach

Add a flag to history.state when hitting the Search toggle in Browse search, and use it to determine whether to use default sorting.

Refs

UIIN-3137

Screenshots

2024-11-29_15h43_06.mp4

…e to use in the condition that specifies whether to use the default sort on mount.
Copy link

github-actions bot commented Nov 29, 2024

Jest Unit Test Statistics

       1 files  ±0     253 suites  ±0   14m 21s ⏱️ -22s
1 015 tests +1  1 013 ✔️ +1  2 💤 ±0  0 ±0 
1 022 runs  +1  1 020 ✔️ +1  2 💤 ±0  0 ±0 

Results for commit bbf9cd1. ± Comparison against base commit 034df62.

♻️ This comment has been updated with latest results.

Copy link

sonarcloud bot commented Dec 2, 2024

@Dmytro-Melnyshyn Dmytro-Melnyshyn merged commit a83e729 into master Dec 2, 2024
5 checks passed
@Dmytro-Melnyshyn Dmytro-Melnyshyn deleted the UIIN-3137 branch December 2, 2024 12:28
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

@Dmytro-Melnyshyn, this PR fixes one bug (losing state when toggling between search and browse) but introduces another in its place (losing state when toggling between this application and another).

The screen-capture shows ui-inventory navigating from its default state [A], (sort: title) to a new state [B] (sort: contributor), out to another app, and then navigating back to ui-inventory. At this point, ui-inventory state should reflect [B], but it's [A]. This is incorrect, and not how other applications act.

Please review the requested functionality with your PO before you merge. I'm happy to be involved in the discussion if there are questions about how application state should be preserved across navigation actions.

@Dmytro-Melnyshyn
Copy link
Contributor Author

@zburke, you are right, we should keep sorting between apps. I reverted this merge #2684

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants