refactor: use URL params to populate breadcrumbs; remove duplicate file share path data from UI state #246
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- since breadcrumbs are now populated from the URL params, no need to make fake data to preserve the crumbs in cases where a 403 error throws
- default behavior is to retry 3 times - this was causing a delay in showing the 403 error message
- this is duplicative of the currentFileSharePath property in fileQuery.data
- remove use of fsp from fileBrowserState
neomorphic
approved these changes
Nov 17, 2025
Member
neomorphic
left a comment
neomorphic
left a comment
There was a problem hiding this comment.
This looks good. Making the changes to remove the fileBrowserState.uiFileSharePath seems to have made it much simpler.
| const { refreshFileBrowser } = useRefreshFileBrowser(); | ||
|
|
||
| const currentFileSharePath = fileBrowserState.uiFileSharePath; | ||
| const currentFileSharePath = fileQuery.data?.currentFileSharePath; |
Member
There was a problem hiding this comment.
Since you are destructuring the fileQuery.data object, you could combine these two lines into a single line like this
const { currentFileSharePath, currentFileOrFolder } = fileQuery.data ?? {};
Collaborator
Author
There was a problem hiding this comment.
Thanks! I changed this and two other locations where I had this same pattern.
Collaborator
Author
|
I'm 99% certain the check links action is failing due to the Cloudfare outage - it's getting a 500 error back from the Material Tailwind docs site |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clickup id: 86ad1x2eg
This PR refactors breadcrumb navigation and state management in the file browser. Breadcrumbs are now populated directly from URL parameters instead of being computed from the navigation state, which simplifies state management and removes the need to create fake data for 403 errors.
In the process of making this change, I realized that
FileBrowserContext.tsxcreated a duplicate of the file share path data fromfileQuery.data.currentFileSharePathinfileBrowserState.uiFileSharePath.fileBrowserStateis meant to only contain UI-specific state - there is no case where the current file share path as computed from the URLfspNameandzonesAndFspQuery.datawould differ from the file share path displayed in the UI. This PR removesuiFileSharePathfromfileBrowserState, and update the components and hooks to either use thefspNamedirectly, where appropriate, or else use the validatedfileQuery.data.currentFileSharePathvalue.Changes
components/ui/BrowsePage/Crumbs.tsx- Populate breadcrumbs from URL parametersqueries/fileQueries.tscontexts/FileBrowserContext.tsx- RemoveuiFileSharePathfromFileBrowserStatefileQueryresult instead offileBrowserStatehooks/useOpenZones.ts- Remove auto-open zone effect when navigating to a file share path in that zone (I didn't think this feature was that useful)