Skip to content

Commit

Permalink
[Asset graph sidebar] Minor improvements (#16729)
Browse files Browse the repository at this point in the history
## Summary & Motivation
- [x]  Set the cursor to `Pointer` on hover on the sidebar items
- [x] [Nit] The active state background color for sidebar items should
be blue.50
- [x] [Nit] Change the arrow icon back to the … icon because it feels a
little odd to have two arrow icons
[Nit] Remove the gray line on the left side of the top level here 
- [x]  [Usability] Replace the query instead of appending it. 
- [x]  Change “repository” to “Code location” in the filter dropdown
- [x] Fix buggy search by using Suggest instead

## How I Tested These Changes
locally in OSS
  • Loading branch information
salazarm authored and clairelin135 committed Sep 26, 2023
1 parent 3717528 commit c7c977c
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const AssetGraphExplorerFilters = React.memo(
const visibleReposSet = React.useMemo(() => new Set(visibleRepos), [visibleRepos]);

const reposFilter = useStaticSetFilter<DagsterRepoOption>({
name: 'Repository',
name: 'Code location',
icon: 'repo',
allValues: allRepos.map((repo) => ({
key: repo.repository.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import {
Menu,
MenuItem,
Popover,
TextInput,
MiddleTruncate,
useViewport,
MenuDivider,
Spinner,
ButtonGroup,
Tooltip,
Suggest,
} from '@dagster-io/ui-components';
import {useVirtualizer} from '@tanstack/react-virtual';
import React from 'react';
Expand Down Expand Up @@ -84,7 +84,7 @@ export const AssetGraphExplorerSidebar = React.memo(
if (!assetGraphData.nodes[id]) {
try {
const path = JSON.parse(id);
const nextOpsQuery = `${explorerPath.opsQuery} \"${tokenForAssetKey({path})}\"`;
const nextOpsQuery = `\"${tokenForAssetKey({path})}\"`;
onChangeExplorerPath(
{
...explorerPath,
Expand Down Expand Up @@ -447,7 +447,7 @@ const Node = ({
const newQuery = `\"${tokenForAssetKey({path})}\"*`;
const nextOpsQuery = explorerPath.opsQuery.includes(newQuery)
? explorerPath.opsQuery
: `${explorerPath.opsQuery} ${newQuery}`;
: newQuery;
onChangeExplorerPath(
{
...explorerPath,
Expand All @@ -462,7 +462,7 @@ const Node = ({
const newQuery = `*\"${tokenForAssetKey({path})}\"`;
const nextOpsQuery = explorerPath.opsQuery.includes(newQuery)
? explorerPath.opsQuery
: `${explorerPath.opsQuery} ${newQuery}`;
: newQuery;
onChangeExplorerPath(
{
...explorerPath,
Expand Down Expand Up @@ -517,7 +517,7 @@ const Node = ({
style={{
width: '100%',
borderRadius: '8px',
...(isSelected ? {background: Colors.LightPurple} : {}),
...(isSelected ? {background: Colors.Blue50} : {}),
}}
>
<div
Expand Down Expand Up @@ -591,7 +591,7 @@ const Node = ({
shouldReturnFocusOnClose
>
<ExpandMore style={{cursor: 'pointer'}}>
<Icon name="expand_more" color={Colors.Gray500} />
<Icon name="more_horiz" color={Colors.Gray500} />
</ExpandMore>
</Popover>
</div>
Expand Down Expand Up @@ -654,7 +654,7 @@ const BoxWrapper = ({level, children}: {level: number; children: React.ReactNode
<Box
padding={{left: 8}}
margin={{left: 8}}
border={{side: 'left', width: 1, color: Colors.KeylineGray}}
border={i > 0 ? {side: 'left', width: 1, color: Colors.KeylineGray} : undefined}
>
{sofar}
</Box>
Expand All @@ -677,70 +677,40 @@ const SearchFilter = <T,>({
values: {label: string; value: T}[];
onSelectValue: (e: React.MouseEvent<any>, value: T) => void;
}) => {
const [searchValue, setSearchValue] = React.useState('');
const filteredValues = React.useMemo(() => {
if (searchValue) {
return values.filter(({label}) => label.toLowerCase().includes(searchValue.toLowerCase()));
}
return values;
}, [searchValue, values]);

const {viewport, containerProps} = useViewport();
return (
<Popover
placement="bottom-start"
targetTagName="div"
content={
searchValue ? (
<Box
style={{
width: viewport.width + 'px',
borderRadius: '8px',
maxHeight: 'min(500px, 50vw)',
overflow: 'auto',
}}
>
<Menu>
{filteredValues.length ? (
filteredValues.map((value) => (
<MenuItem
key={value.label}
onClick={(e) => {
onSelectValue(e, value.value);
setSearchValue('');
}}
text={value.label}
/>
))
) : (
<Box padding={8}>No results</Box>
)}
</Menu>
</Box>
) : (
<div />
)
}
>
<div style={{display: 'grid', gridTemplateColumns: '1fr'}}>
<TextInput
icon="search"
value={searchValue}
onChange={(e) => {
setSearchValue(e.target.value);
}}
placeholder="Search assets"
{...(containerProps as any)}
/>
</div>
</Popover>
<SuggestWrapper {...containerProps}>
<Suggest<(typeof values)[0]>
key="asset-graph-explorer-search-bar"
inputProps={{placeholder: 'Search', style: {width: `min(100%, ${viewport.width}px)`}}}
items={values}
inputValueRenderer={(item) => item.label}
itemPredicate={(query, item) =>
item.label.toLocaleLowerCase().includes(query.toLocaleLowerCase())
}
menuWidth={viewport.width}
popoverProps={{usePortal: false, fill: true}}
itemRenderer={(item, itemProps) => (
<MenuItem
active={itemProps.modifiers.active}
onClick={(e) => itemProps.handleClick(e)}
key={item.label}
text={item.label}
/>
)}
noResults={<MenuItem disabled={true} text="No results" />}
onItemSelect={(item, e) => onSelectValue(e as any, item.value)}
selectedItem={null}
/>
</SuggestWrapper>
);
};

const ExpandMore = styled.div``;

const GrayOnHoverBox = styled(Box)`
border-radius: 8px;
cursor: pointer;
&:hover {
background: ${Colors.Gray100};
transition: background 100ms linear;
Expand All @@ -766,3 +736,9 @@ const ButtonGroupWrapper = styled.div`
function nodeId(node: {path: string; id: string} | {id: string}) {
return 'path' in node ? node.path : node.id;
}

const SuggestWrapper = styled.div`
.bp4-input-group.dagster-suggest-input {
width: 100%;
}
`;

0 comments on commit c7c977c

Please sign in to comment.