-
Notifications
You must be signed in to change notification settings - Fork 9
Add compact icon-based view switcher to the data page toolbar #530
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
base: master
Are you sure you want to change the base?
Changes from all commits
e04694d
ab78da3
8feff5f
75e185c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -200,15 +200,13 @@ const FilterPath: React.FC = () => { | |
| if (filters.filter((f) => f).length === 0) { | ||
| return ( | ||
| <> | ||
| <SlashIcon size="1em" /> | ||
| <Deemphasized>No filters applied</Deemphasized> | ||
| </> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <SlashIcon size="1em" /> | ||
|
Comment on lines
208
to
-211
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here too |
||
| {filters | ||
| .filter((f) => f) | ||
| .map((filter, i) => ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,47 +1,53 @@ | ||
| import { SlidersHorizontalIcon } from 'lucide-react'; | ||
| import React, { Suspense } from 'react'; | ||
|
|
||
| import useFilterPanel from '@widgets/controls/useFilterPanel'; | ||
| import FilterPanelToggle from '@widgets/controls/FilterPanelToggle'; | ||
| import ViewSelector from '@widgets/controls/selectors/ViewSelector'; | ||
| import Loading from '@widgets/Loading'; | ||
| import PathNav from '@widgets/pathnav/PathNav'; | ||
| import { PathContainer } from '@widgets/pathnav/PathNav'; | ||
|
|
||
| import HoverableButton from '@features/layers/hovercard/HoverableButton'; | ||
| import ResultCount from '@features/pagination/ResultCount'; | ||
| import FilterPath from '@features/transforms/filtering/FilterPath'; | ||
| import SearchBar from '@features/transforms/search/SearchBar'; | ||
| import SortBySelector from '@features/transforms/sorting/SortBySelector'; | ||
|
|
||
| import EntityTypeTabs from './dataviews/EntityTypeTabs'; | ||
|
|
||
| const DataViews = React.lazy(() => import('./dataviews/DataViews')); | ||
|
|
||
| const DataPageBody: React.FC = () => { | ||
| const { isOpen, setIsOpen } = useFilterPanel(); | ||
|
|
||
| return ( | ||
| <main style={{ padding: '1em', flex: 1, overflow: 'auto', width: '100%' }}> | ||
| <div style={{ display: 'flex', alignItems: 'center', flexDirection: 'column' }}> | ||
| <SearchBar /> | ||
| <EntityTypeTabs /> | ||
| </div> | ||
|
|
||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The alignment doesn't line up between the right and left sides. |
||
| justifyContent: 'space-between', | ||
| width: '100%', | ||
| marginBottom: '1rem', | ||
| }} | ||
| > | ||
| <div style={{ display: 'flex', alignItems: 'center', gap: '0.5em' }}> | ||
| <FilterPanelToggle /> | ||
| <ResultCount /> | ||
| <PathContainer> | ||
| <FilterPath /> | ||
| </PathContainer> | ||
| </div> | ||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'space-between', | ||
| width: '100%', | ||
| justifyContent: 'flex-end', | ||
| gap: '0.5rem', | ||
| }} | ||
| > | ||
| <div style={{ display: 'flex', alignItems: 'center', gap: '0.5em' }}> | ||
| <HoverableButton | ||
| className={isOpen ? 'selected primary' : 'primary'} | ||
| hoverContent={isOpen ? 'Close filters panel' : 'Open filters panel'} | ||
| onClick={() => setIsOpen((open) => !open)} | ||
| style={{ padding: '0.4em', borderRadius: '0.5em', display: 'flex' }} | ||
| aria-label={isOpen ? 'Close filters panel' : 'Open filters panel'} | ||
| > | ||
| <SlidersHorizontalIcon size="1.2em" /> | ||
| </HoverableButton> | ||
| <ResultCount /> | ||
| </div> | ||
| <PathNav /> | ||
| <SortBySelector showLabel={false} /> | ||
| <ViewSelector /> | ||
| </div> | ||
| </div> | ||
| <div | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { SlidersHorizontalIcon } from 'lucide-react'; | ||
|
|
||
| import HoverableButton from '@features/layers/hovercard/HoverableButton'; | ||
|
|
||
| import useFilterPanel from './useFilterPanel'; | ||
|
|
||
| const FilterPanelToggle: React.FC = () => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it got left out before the merge. Added now. |
||
| const { isOpen, setIsOpen } = useFilterPanel(); | ||
| const label = isOpen ? 'Close filters panel' : 'Open filters panel'; | ||
|
|
||
| return ( | ||
| <HoverableButton | ||
| className={isOpen ? 'selected primary' : 'primary'} | ||
| hoverContent={label} | ||
| onClick={() => setIsOpen((open) => !open)} | ||
| style={{ padding: '0.5em', display: 'flex' }} | ||
| aria-label={label} | ||
| > | ||
| <SlidersHorizontalIcon size="1.2em" /> | ||
| </HoverableButton> | ||
| ); | ||
| }; | ||
|
|
||
| export default FilterPanelToggle; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { ChartColumnBigIcon, Grid2x2Icon, ListTreeIcon, MapIcon, Table2Icon } from 'lucide-react'; | ||
| import React from 'react'; | ||
|
|
||
| import { View } from '@features/params/PageParamTypes'; | ||
|
|
@@ -10,25 +11,36 @@ const ViewSelector: React.FC = () => { | |
|
|
||
| return ( | ||
| <Selector | ||
| selectorLabel="Display" | ||
| getOptionDescription={(option) => <img src={getImageSrc(option)} width={180} />} | ||
| options={Object.values(View)} | ||
| onChange={(view: View) => updatePageParams({ view })} | ||
| display={SelectorDisplay.ButtonList} | ||
| getOptionLabel={(view) => | ||
| [View.Map, View.Reports].includes(view) ? ( | ||
| <> | ||
| {view} <em>β</em> | ||
| </> | ||
| ) : ( | ||
| view | ||
| ) | ||
| } | ||
| selected={view} | ||
| onChange={(nextView: View) => updatePageParams({ view: nextView })} | ||
| getOptionLabel={(option) => getViewIcon(option)} | ||
| getOptionDescription={(option) => getViewLabel(option)} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see you removed the screenshots from the mouseover -- that was intentional because you want new screenshots?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have added Screenshots back in the mouseover descriptions along with the view label. |
||
| display={SelectorDisplay.ButtonGroup} | ||
| optionStyle={{ | ||
| width: 'fit-content', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| padding: '0.5rem', | ||
| }} | ||
| selectorStyle={{ gap: '0.25rem' }} | ||
|
Comment on lines
+20
to
+27
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| /> | ||
| ); | ||
| }; | ||
|
|
||
| function getViewLabel(view: View): React.ReactNode { | ||
| const isBeta = [View.Map, View.Reports].includes(view); | ||
| return ( | ||
| <div style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', gap: '0.25rem' }}> | ||
| <span style={{ display: 'flex', gap: '0.125rem' }}> | ||
| {view} {isBeta && <em>β</em>} | ||
| </span> | ||
| <img src={getImageSrc(view)} width={180} /> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function getImageSrc(view: View): string { | ||
| switch (view) { | ||
| case View.CardList: | ||
|
|
@@ -44,4 +56,19 @@ function getImageSrc(view: View): string { | |
| } | ||
| } | ||
|
|
||
| function getViewIcon(view: View): React.ReactNode { | ||
| switch (view) { | ||
| case View.CardList: | ||
| return <Grid2x2Icon size="1.2em" />; | ||
| case View.Hierarchy: | ||
| return <ListTreeIcon size="1.2em" />; | ||
| case View.Map: | ||
| return <MapIcon size="1.2em" />; | ||
| case View.Table: | ||
| return <Table2Icon size="1.2em" />; | ||
| case View.Reports: | ||
| return <ChartColumnBigIcon size="1.2em" />; | ||
| } | ||
| } | ||
|
|
||
| export default ViewSelector; | ||
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.
You can remove the <></> syntax since its only one component