feat(project-tree): new page breadcrumbs #31600
Conversation
|
Size Change: +334 B (0%) Total Size: 13.6 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
PR Summary
This PR implements consistent breadcrumb navigation for 'new' pages across the application by modifying the projectTreeRef selector behavior in various logic files.
- Modified
ProjectTreeRefinterface intypes.tsto allownullvalues forrefproperty - Updated
projectTreeRefselectors across 10 logic files to returnnullinstead of string conversion when handling 'new' items - Added new
projectTreeRefselector tocampaignsLogic.tsandpipelineNodeNewLogic.tsxfor breadcrumb support - Standardized breadcrumb handling for 'new' pages to improve folder organization in project tree navigation
- Maintained consistent URL handling and state management patterns across all modified files
10 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
| (experimentId): ProjectTreeRef => { | ||
| return { type: 'experiment', ref: experimentId === 'new' ? null : String(experimentId) } | ||
| }, |
There was a problem hiding this comment.
style: Consider adding a type guard to ensure experimentId is either 'new' or a valid experiment ID
| projectTreeRef: [ | ||
| (s) => [s.loading], | ||
| (): ProjectTreeRef => ({ | ||
| type: 'hog_function/', | ||
| ref: null, | ||
| }), | ||
| ], |
There was a problem hiding this comment.
logic: The projectTreeRef selector returns a hardcoded type 'hog_function/' with null ref, but doesn't use the loading state it depends on. Either remove the unused loading dependency or implement proper loading state handling.
| projectTreeRef: [ | |
| (s) => [s.loading], | |
| (): ProjectTreeRef => ({ | |
| type: 'hog_function/', | |
| ref: null, | |
| }), | |
| ], | |
| projectTreeRef: [ | |
| () => [], | |
| (): ProjectTreeRef => ({ | |
| type: 'hog_function/', | |
| ref: null, | |
| }), | |
| ], |
| type: 'hog_function/', | ||
| ref: null, |
There was a problem hiding this comment.
style: The hardcoded 'hog_function/' type may not be appropriate for all pipeline nodes. Consider deriving the type from the stage or kind props.
| projectTreeRef: [ | ||
| (s) => [s.campaignId], | ||
| (id): ProjectTreeRef => ({ | ||
| type: 'hog_function/campaign', |
There was a problem hiding this comment.
style: The hardcoded type string 'hog_function/campaign' should be defined as a constant to avoid typos and ensure consistency across the codebase
| (s) => [s.campaignId], | ||
| (id): ProjectTreeRef => ({ | ||
| type: 'hog_function/campaign', | ||
| ref: id === 'new' ? null : id, | ||
| }), |
There was a problem hiding this comment.
style: The projectTreeRef selector reuses the campaignId selector but renames the parameter to 'id' - consider keeping the parameter name consistent as 'campaignId' for clarity
Changes
Adds breadcrumbs with the selected folder for all "new" pages.
How did you test this code?
👀