[ui] For runs targeting assets, show >1 asset in the Target column #26122
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 5d19ed5. |
bengotow
force-pushed
the
bengotow-2024-11/FE702
branch
from
dfe7d60
to
cefdda4
Compare
We’ve had reports that users who launch small N numbers of assets together find the new run feed UI difficult to use, since it shows either “asset name” or “2 assets” in the Target column. This PR: - Expands the target column a bit - Shows as many asset / check tags as will fit in the available space, and then “X more” The implementation of this uses a new hook `useAdjustChildVisibilityToFill`. The idea is that your component renders some reasonable max number of tags (10) and a more tag, and the hook uses a resize observer + layout effect to show/hide the tags to fit the available space. I tried doing this using React state, but it looks bad if you can see it adding / removing tags. I think the other approach would be to write a tag “measure” function, or otherwise repeatedly render + size them offscreen, but that’d still force layouts, and in this case the tags are not identical react components (the “4 more” tag is slightly different) I added a storybook that makes it easy to test what this looks like in a bunch of scenarios. Sidenote: There’s a bunch of cruft here because the “Target” column components all have to support a “tags” rendering and a “plain” rendering. This is going away soon when we remove the FF allowing users to revert to the old runs page, and I think it’ll clean up this code!
bengotow
force-pushed
the
bengotow-2024-11/FE702
branch
from
cefdda4
to
5d19ed5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We’ve had reports that users who launch small N numbers of assets together find the new run feed UI difficult to use, since it shows either “asset name” or “2 assets” in the Target column. The "2 assets" form makes it difficult to scan for a run you have in mind:
This PR:
The implementation of this uses a new hook
useAdjustChildVisibilityToFill. The idea is that your component renders some reasonable max number of tags (10) and a more tag, and the hook uses a resize observer + layout effect to show/hide the tags to fit the available space. I tried doing this using React state, but it looks bad because you can see it adding / removing tags, especially as you resize the viewport. I think the other approach would be to write a tag “measure” function, or otherwise repeatedly render + size them offscreen, but that’d still force layouts with the added cost of a React.render (the tags are not identical react components - the “4 more” tag is slightly different - so I think the offscreen approach would need to actually render the tag...) The text + size of the "more" tag also changes based on the number of tags displayed.Sidenote: There’s a bunch of cruft here because the “Target” column components all have to support a “tags” rendering and a “plain” rendering. This is going away soon when we remove the FF allowing users to revert to the old runs page, and I think it’ll clean up this code!
How I Tested These Changes
I added a storybook that makes it easy to test what this looks like in a bunch of scenarios and verify it works nicely.
Screen.Recording.2024-11-24.at.12.21.15.PM.mov
Before:
After:
Related: https://linear.app/dagster-labs/issue/FE-702/show-more-than-one-asset-tag-on-the-new-runs-feed-page