-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix: Open empty action sidebar on back navigation #4154
Conversation
Fix: Open empty action sidebar on back navigation
2c7b7ff
to
4fa9c23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, this is working great. I like the solution.
Testing it out, everything works as I assume it would:
Screen.Recording.2025-01-24.at.10.47.36.mov
Testing the colony creation flow:
Screen.Recording.2025-01-24.at.10.48.59.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -62,10 +62,15 @@ const ActionSidebarContextProvider: FC<PropsWithChildren> = ({ children }) => { | |||
const selectedDomain = useGetSelectedDomainFilter(); | |||
const selectedDomainNativeId = selectedDomain?.nativeId ?? ''; | |||
const navigate = useNavigate(); | |||
const [searchParams] = useSearchParams(); | |||
const transactionId = searchParams?.get(TX_SEARCH_PARAM); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small comment here: we can rename transactionId to have "param" in its naming for more clarity at 70 line 🙌 But it is not necessary if you don't feel doing it 🫶
const transactionId = searchParams?.get(TX_SEARCH_PARAM); | |
const transactionIdParam = searchParams?.get(TX_SEARCH_PARAM); |
Fix: Open empty action sidebar on back navigation
Description
Redo action
opens an empty action sidebar, even though this is no longer reproducible.Screen.Recording.2025-01-23.at.11.50.20.mov
Screen.Recording.2025-01-23.at.12.01.24.mov
This was mainly caused by the fact that on back navigation, if the action sidebar was opened, we didn't react to the absence of a
txHash
andactionType
. So my solution involved listening to location key changes (thekey
should update once the navigation history updates) and if there is notxHash
andactionType
present, to close the action sidebar.Additionally, we were mutating the history state needlessly by always performing a navigation without the
txHash
upon closing the action sidebar, even if there was notxHash
set, so I have fixed that.Important
If you do find a better solution or approach, please do reach out
On this branch
Screen.Recording.2025-01-23.at.13.53.07.mov
Testing
TODO: Let's test the browser back navigation no longer opens an empty action sidebar.
create-colony-url
script. Complete the colony creation form.Recent Activity
tableCreate new action
button. Check it still opens the empty action sidebar.Please try also whatever other scenario comes to mind.
Diffs
New stuff ✨
useLocationKeyChange
hookChanges 🏗
actionSidebarInitialValues?.[ACTION_TYPE_FIELD_NAME]
in theColonyLayout
txHash
and perform the navigation on action sidebar close only if it exists inActionSidebarContextProvider
Resolves #3890