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

Correctly estimate row heights in sequence table #4393

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

rpiaggio
Copy link
Contributor

This avoids the flash when scrolling.

@mergify mergify bot added the explore label Dec 17, 2024
Copy link

bundlemon bot commented Dec 17, 2024

BundleMon

Files updated (1)
Status Path Size Limits
index-(hash).js
1.71MB (+1.01KB +0.06%) -
Unchanged files (7)
Status Path Size Limits
exploreworkers-(hash).js
594.61KB -
index-(hash).css
66.06KB -
workbox-window.prod.es5-(hash).js
2.07KB -
catalogworker-(hash).js
90B -
plotworker-(hash).js
88B -
agsworker-(hash).js
87B -
itcworker-(hash).js
87B -

Total files change +1.01KB +0.04%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

Copy link
Contributor

@toddburnside toddburnside left a comment

Choose a reason for hiding this comment

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

LGTM

@cquiroz
Copy link
Contributor

cquiroz commented Dec 17, 2024

Isn't this common with observe? Should it be fixed in lucuma-ui?

@rpiaggio
Copy link
Contributor Author

rpiaggio commented Dec 17, 2024

Isn't this common with observe? Should it be fixed in lucuma-ui?

Observe uses a lot of common code but renders the sequence a bit different: it has a CurrentRow for special rendering of the currently loaded row, and it renders visit headers.

However, I just realized that the heights are determined in lucuma-ui, so maybe we should have constants there that observe and explore can use. I'll refactor.

Copy link
Contributor

@cquiroz cquiroz left a comment

Choose a reason for hiding this comment

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

LGTM

@rpiaggio rpiaggio merged commit ec557c7 into master Dec 17, 2024
14 checks passed
@rpiaggio rpiaggio deleted the sc-4307-flashing-sequence-table-when-scrolling branch December 17, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants