Add compact icon-based view switcher to the data page toolbar#530
Add compact icon-based view switcher to the data page toolbar#530
Conversation
Expose optionStyle on Selector and merge it into SelectorOption so options can be styled. Add a compact icon-based ViewSelector (uses lucide-react), place it in DataPageBody, and remove the inline ViewSelector from PathNav.
Add optional showLabel prop to SortBySelector (defaults to true)
so callers can omit its label/description; DataPageBody now passes
showLabel={false} for the top toolbar. Adjust ViewSelector optionStyle
to use fit-content width and increased padding for better button sizing.
conradarcturus
left a comment
There was a problem hiding this comment.
Some style changes but the functionality generally looks good.
| <Selector | ||
| selectorLabel="Sort By" | ||
| selectorDescription="Choose the order of items in the view." | ||
| selectorLabel={showLabel ? 'Sort By' : undefined} |
There was a problem hiding this comment.
When not showing the text maybe we should show ArrowDownUpIcon?
There was a problem hiding this comment.
Done, now shows ArrowDownUpIcon as the label when showLabel is false
src/widgets/pathnav/PathNav.tsx
Outdated
| import usePageParams from '@features/params/usePageParams'; | ||
| import FilterPath from '@features/transforms/filtering/FilterPath'; | ||
|
|
||
| const PathNav: React.FC = () => { |
There was a problem hiding this comment.
The PathNav component is almost completely empty now. It can probably be simplified.
Also can you remove the initial SlashIcon from FilterPath? It's not necessary since the selectors before it (Entity & View) have both been moved.
There was a problem hiding this comment.
Simplified PathNav.tsx to only export PathContainer, DataPageBody now uses PathContainer + FilterPath directly.
| return <Grid2x2Icon size="1.2em" />; | ||
| case View.Hierarchy: | ||
| return '/lang-nav/hierarchy.png'; | ||
| return <GitBranchIcon size="1.2em" />; |
There was a problem hiding this comment.
NetworkIcon may be better? But I'll let you decide.
There was a problem hiding this comment.
Went with ListTreeIcon instead, feels more intuitive for a language hierarchy.
There was a problem hiding this comment.
oh yes ListTreeIcon is even better! Good find.
src/pages/DataPageBody.tsx
Outdated
| </HoverableButton> | ||
| <ResultCount /> | ||
| </div> | ||
| <PathNav /> |
There was a problem hiding this comment.
Since PathNav is now just the "ActiveFilters" we should probably put it on the left side, not the right side.
There was a problem hiding this comment.
Moved active filters to the left side.
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| alignItems: 'center', |
There was a problem hiding this comment.
The alignment doesn't line up between the right and left sides.
|
|
||
| import useFilterPanel from './useFilterPanel'; | ||
|
|
||
| const FilterPanelToggle: React.FC = () => { |
There was a problem hiding this comment.
Yes, this is a good idea. I'm a bit confused why it wasn't part of your previously merged commit -- But I guess you merged before you could get it in.
There was a problem hiding this comment.
Yeah, it got left out before the merge. Added now.
| optionStyle={{ | ||
| width: 'fit-content', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| padding: '0.5rem', | ||
| }} | ||
| selectorStyle={{ gap: '0.25rem' }} |
There was a problem hiding this comment.
TODO for me: check if we need all of these styles, I'm not certain (ideally it would come from SelectorDisplay so it scales better for more usages).
Nonetheless we don't need to get ahead of ourselves and scale things that we are only using in one place right now.
| selected={view} | ||
| onChange={(nextView: View) => updatePageParams({ view: nextView })} | ||
| getOptionLabel={(option) => getViewIcon(option)} | ||
| getOptionDescription={(option) => getViewLabel(option)} |
There was a problem hiding this comment.
I see you removed the screenshots from the mouseover -- that was intentional because you want new screenshots?
There was a problem hiding this comment.
I have added Screenshots back in the mouseover descriptions along with the view label.
conradarcturus
left a comment
There was a problem hiding this comment.
There are a few singletons in fragments that you can remove the fragments <></>
Otherwise it looks good!
| return ( | ||
| <> | ||
| <SlashIcon size="1em" /> | ||
| <Deemphasized>No filters applied</Deemphasized> | ||
| </> | ||
| ); |
There was a problem hiding this comment.
You can remove the <></> syntax since its only one component
| return ( | ||
| <> | ||
| <SlashIcon size="1em" /> |
Fixes #485, #486
Summary: Reworks the data page controls so view switching is faster and more compact. It replaces the text-based display selector with an icon-based control, pulls the sort selector into the top toolbar, and extracts the filter toggle into its own reusable component.
Changes
toLocaleString()for better readability on large result sets.optionStylesupport to the shared selector components so button-like selectors can customize layout without duplicating logic.showLabelprop toSortBySelectorso it can be reused in tighter toolbar layouts.src/widgets/controls/FilterPanelToggle.tsx.src/widgets/pathnav/PathNav.tsx.src/pages/DataPageBody.tsxby composing dedicated controls instead of inline toggle logic.Out of scope/Future work: Add screenshots and manual QA notes after testing the updated toolbar across desktop and mobile layouts.
Test Plan and Screenshots
How to test the changes in this PR:
npm run dev.Checklist
Summary
Testing
npm run lintnpm run buildnpm run testnpm run dev-- tried out the website directlyInternal changes
Docs