Skip to content
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

fix(breadcrumbs): Fix flicking on scroll from bottom to top - [TET-429] #41055

Merged
merged 1 commit into from
Nov 10, 2022

Conversation

priscilawebdev
Copy link
Member

@priscilawebdev priscilawebdev commented Nov 7, 2022

Issue:

In the breadcrumbs of an issue, we currently use components from the react-virtualized library to render a virtual list.

The current logic makes the UI automatically scroll to the last item in the list when the component is rendered and if users want to see more items from the list they have to scroll from the bottom to the top.

The problem is that since the items have a dynamic height and they haven't been rendered yet, it causes the list to have a flickering effect which is very annoying.

This issue seems to be well-known by the library's maintainers and according to some discussions, it is yet not fixed. See:

bvaughn/react-virtualized#610

The same issue seems to be happening in the new version of the library too:

bvaughn/react-window#6

Solution (workaround):

As the virtual list seems to work fine when scrolling from top to bottom, the workaround was to invert the list items and also the CSS and this seems to present a better user experience.

Before:

Screen.Recording.2022-11-07.at.12.30.52.mov

After:

Screen.Recording.2022-11-07.at.12.29.28.mov

ps: took the opportunity to do some code cleanup

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Nov 7, 2022
@priscilawebdev priscilawebdev changed the title fix(breadcrumbs): Fix flicking on scroll from bottom to top fix(breadcrumbs): Fix flicking on scroll from bottom to top - [TET-429] Nov 7, 2022
@priscilawebdev priscilawebdev marked this pull request as ready for review November 7, 2022 11:32
@priscilawebdev priscilawebdev requested a review from a team November 7, 2022 11:33
@priscilawebdev priscilawebdev requested review from a team as code owners November 7, 2022 11:33

const Wrapper = styled('div')<{error: boolean; scrollbarSize: number}>`
display: grid;
transform: scaleY(-1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flips here

@@ -200,10 +175,11 @@ const StyledPanelTable = styled(PanelTable)<{scrollbarSize: number}>`

/* Content */
:nth-child(n + 7) {
${p => !p.isEmpty && 'transform: scaleY(-1)'}; /* Flip the list */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flips here

@priscilawebdev priscilawebdev merged commit a9e6502 into master Nov 10, 2022
@priscilawebdev priscilawebdev deleted the ref/breadcrumbs-fix-lagging-on-scroll branch November 10, 2022 07:45
@priscilawebdev priscilawebdev added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Nov 11, 2022
@getsentry-bot
Copy link
Contributor

PR reverted: 5c05c22

getsentry-bot added a commit that referenced this pull request Nov 11, 2022
…#41055)"

This reverts commit a9e6502.

Co-authored-by: priscilawebdev <29228205+priscilawebdev@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Nov 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components Trigger: Revert Add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants