Merged
Conversation
This is an UX improvement to keep the user focused on the card they're viewing when they scroll down a large list of labels.
bl-nero
requested review from
rosstimothy
and removed request for
ibeckermayer and
ravicious
Contributor
Author
|
Context: this was one part of @rosstimothy 's feedback that I had at the back of my head, and it was bugging me for a while. I'm not sure if this addresses the concern, so I'm awaiting your judgement here. :) |
avatus
approved these changes
Sep 12, 2023
Contributor
avatus
left a comment
avatus
left a comment
There was a problem hiding this comment.
Works as intended after testing. I like it!
Comment on lines
+152
to
+156
| // If the user expanded the labels and then scrolled down enough to hide the | ||
| // top of the card, we scroll back up and collapse the labels with a small | ||
| // delay to keep the user from losing focus on the card that they were | ||
| // looking at. The delay is picked by hand, since there's no (easy) way to | ||
| // know when the animation ends. |
Contributor
There was a problem hiding this comment.
I came to the PR here looking for "why is there a delay for this card closing?" because it was jarring to me, but after reading this comment, I agree with the approach. Thanks for changing my mind
ibeckermayer
approved these changes
Sep 13, 2023
public-teleport-github-review-bot
Bot
removed the request for review
from rosstimothy
avatus
pushed a commit
that referenced
this pull request
Sep 15, 2023
This is an UX improvement to keep the user focused on the card they're viewing when they scroll down a large list of labels.
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Sep 16, 2023
* Ad a storybook for the Unified Resources screen (#31213) * Add a mock service worker Storybook add-on * Add a Resources story * Add the license * Lint fix * Scroll up when collapsing labels (#31755) This is an UX improvement to keep the user focused on the card they're viewing when they scroll down a large list of labels. * Only show copy button on hover (#31920) --------- Co-authored-by: Bartosz Leper <bartosz.leper@goteleport.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This is an UX improvement to keep the user focused on the card they're viewing when they scroll down a large list of labels.
Demo: https://www.loom.com/share/f37b52fa0c054419aad336df6d142fe8