Add global sort and filter indicators to the Experiments Table webview #1872
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
|
This is really good. Could add an onClick action to the button to show the specific tree. E.g That would lead the user back the view container where they can see/perform all of the required actions. Could be enough to bridge the gap between what is currently in |
|
It definitely looks cool, but I'm not sure that it solves the problem though. Question- what do those numbers mean? Number of filters / sorts applied? I was expecting that we would put something like "10 experiments are hidden ". Hidden could be a link to activate the filters view. |
|
Adding more text would now be duplicating information in the UI:
Filtered experiments/checkpoints numbers are shown at the top of the filter by tree and the records are greyed out in the experiments tree. |
Do you mean the side panel? We can't expect it to be open. Table should be informative enough and have enough information to see its state more or less. Icons are also duplicating the side panel. Also, they are duplicating icons that are making in headers (if columns are not hidden, which should be a rare case I think - filtering / sorting by a hidden column). |
Unless we a going to add a ribbon (and I don't think we should) we should be encouraging users to have the "controls" open. Adding the open view action to the new buttons would do this.
Columns which are being used to sort/filter could also be off-screen. E.g 200 columns in the table. This being in the sticky column could help that. |
It is, yes, but this value is not enough to solve the core issue. The core issue was that there are hidden items and we don't know about them anything. Unfortunately, this implementation won't move the needle for this much.
It't not realistic. We need something (that why we are doing this text, at least I though about it this way). Ribbon or not- some UX / planning is needed. |
With the badge numbers representing the amount of sorts/filters added, this solves only the worst-case scenario of "User has filters applied and doesn't know, the sidebar controls aren't open, and it seems like the extension is broken."
I was thinking that sort of info could be represented in a tooltip that appears when hovering over the filters icon, since the info of "X/Y experiments filtered and A/B checkpoints filtered" is much more difficult to represent in a compact way that works well in this sort of indicator. That said, what @mattseddon is saying about duplicate information is a valid point, such that making a button click focus the sidebar controls could serve as a more minimal solution to the problem, with roughly the following process:
Tooltips would add an alternate and/or middle step between 2 and 3, but I'm not sure how worthwhile it if we have click-to-focus-sidebar would be considering the user would already have their mouse hovered over the indicator at that point and be a click away from seeing the thing closest to the source of truth. |
|
Turns out I didn't look at our custom API enough before making this comment, I think I can get it in this PR today. |
| }) => { | ||
| return ( | ||
| <div className={styles.tableIndicators}> | ||
| <Indicator |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
| > | ||
| <Icon width={16} height={16} icon={SvgSortPrecedence} /> | ||
| </Indicator> | ||
| <Indicator |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
| aria-label="sorts" | ||
| onClick={focusSortsTree} | ||
| > | ||
| <Icon width={16} height={16} icon={SvgSortPrecedence} /> |
There was a problem hiding this comment.
There is an AllIcon const with all the icons in there (needs to add each one manually). I added this because it was an easy way to see all the available icons, but importing them directly like you did is way more convenient. The only thing I would say about this is that it'd be better to import it as SortPrecedence from the icons/index.ts file. Or we could change the SVGR script not to generate an index file, I don't really mind. I'll do a follow up removing AllIcons and importing the icons directly.
|
@rogermparent tests seem genuinely broken. |
|
Tooltips have just been added and pushed with tests! The current format is |
rogermparent
changed the title
|
Happy to merge if the approved diffs in the Storybook are expected. |
| @@ -46,7 +47,8 @@ export default { | |||
| design: { | |||
| type: 'figma', | |||
| url: 'https://www.figma.com/file/AuQXbrFj60xA2QXOjo9Z65/Experiments-Panel-%E2%80%A2-496' | |||
| } | |||
| }, | |||
| layout: 'fullscreen' | |||
There was a problem hiding this comment.
This allows the table stories to work with the position:fixed, as Storybook adds a padding to its iframe by default which is considered part of the viewport which messes with the table otherwise.
|
The big storybook diffs are expected because the Experiments table needed to have its padding disabled to make the indicators work on it, as it's positioned with |
|
Code Climate has analyzed commit 542dd7c and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.6%. View more on Code Climate. |
| <div> | ||
| {formatFilteredCountMessage( | ||
| 'checkpoint', | ||
| filteredCounts.checkpoints |
There was a problem hiding this comment.
[B] @rogermparent are we always showing this? Even when the project doesn't have checkpoints?
There was a problem hiding this comment.
Ah, I think so unless there's some special logic in the filter collector. I can do a quick fix to not show the line when 0 checkpoints are filtered.
This PR adds indicators to the Experiments Table webview that show if the user has any sorts or filters applied. We initially discussed using text mostly for ease of implementation, but that proved pretty hard to fit in so I opted for an icon+badge design similar to VSCode's Activity Bar but a bit smaller.
These indicators are also clickable, doing so will focus the TreeView the represents the same data, either sorts or filters. This will help users see details about the applied filters that these indicators show basic at-a-glance info for, allowing for ease of use while minimizing duplication.
The colors don't exactly match the activity bar because some light themes (like the default) use a light-on-dark scheme for the activity bar that doesn't translate into the table without an extra background for the indicators alone or a drastic change to the table headers- both are still on the table, but I opted for the path of least resistance and went with our normal foreground and watermark colors that are built to contrast with standard backgrounds.
indicators-demo.mp4
These indicators also have tooltips that more explicitly display the amount of sorts/filters added, and in the case of filters the amount of experiments/sorts that have been filtered.
indicator-tooltip-demo.mp4