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

add estimated size to Search Results virtual list in entity picker #53226

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

npfitz
Copy link
Contributor

@npfitz npfitz commented Feb 5, 2025

Closes ENG 8233

Description

Adds an accurate estimated size for a <ChunkyListItem> in the search tab, which solves the issue of of weird scrolling

How to verify

  1. Open an entity picker (data picker is a good choice)
  2. Perform a search that will return many results
  3. grab the scroll bar in the search results, and drag down. the bar should follow your mouse

Demo

Before (on stats):
chrome_gVQ0ofNIVf

After:
chrome_S9FlcS6mii

Checklist

  • [ ] Tests have been added/updated to cover changes in this PR

There was a reference in the original issue that points to a test that can be changed once this is fixed. However, that test has since changed, and doing a simple scrollTo("bottom") still results in weird behavior. I've confirmed the UX is better for real users, but cypress interactions still appear to be fussy

@npfitz npfitz added the backport Automatically create PR on current release branch on merge label Feb 5, 2025
@npfitz npfitz requested a review from kamilmielnik February 5, 2025 14:28
@npfitz npfitz self-assigned this Feb 5, 2025
@npfitz npfitz requested a review from a team February 5, 2025 14:28
@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label Feb 5, 2025
Copy link
Contributor

github-actions bot commented Feb 5, 2025

e2e tests failed on 8f933f6a3f0fe1713be9e6e32730b6fe2dc7d280-1

e2e test run

File Test Name
model-indexes.cy.spec.js (flaky) scenarios > model indexes > should not reload the model for record in the same model
command-palette.cy.spec.js (flaky) command palette > should render a searchable command palette
reproductions-1.cy.spec.js (flaky) issues 11914, 18978, 18977, 23857 > should not display query editing controls and 'Browse databases' link
search-pagination.cy.spec.js (flaky) scenarios > search > should allow users to paginate results

Copy link

trunk-io bot commented Feb 5, 2025

Static BadgeStatic BadgeStatic BadgeStatic Badge

Flaky Test Failure Summary Logs
scenarios > search should allow users to paginate results Unable to find an accessible element with the role "option" and name "/View and filter/" within 4000ms. Logs ↗︎
issues 11914, 18978, 18977, 23857 should not display query editing controls and 'Browse databases' link The test timed out while trying to find an element with the text "Repro" in the UI. Logs ↗︎
command palette should render a searchable command palette Unable to find an accessible element with the role "option" and name "Orders, Count" within a certain time limit. Logs ↗︎
scenarios > model indexes should not reload the model for record in the same model Unable to find an accessible element with the role "option" and name "Small Marble Shoes" within 4000ms. Logs ↗︎

View Full Report ↗︎Docs

@npfitz npfitz merged commit b09343c into master Feb 6, 2025
148 of 157 checks passed
@npfitz npfitz deleted the eng-8233-jammy-scrollbar-in-the-entity-picker-modal branch February 6, 2025 00:24
github-automation-metabase added a commit that referenced this pull request Feb 6, 2025
@github-actions github-actions bot added this to the 0.53 milestone Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/AdminWebapp Admin and Webapp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants