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 layout shift in home component #14112

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Fix layout shift in home component #14112

merged 2 commits into from
Jun 18, 2024

Conversation

pleek91
Copy link
Contributor

@pleek91 pleek91 commented Jun 18, 2024

Closes #14111

@pleek91 pleek91 requested a review from a team as a code owner June 18, 2024 16:01
@pleek91 pleek91 added fix A fix for a bug in an existing feature ui Related to the Prefect web interface labels Jun 18, 2024
Copy link
Contributor

@collincchoy collincchoy left a comment

Choose a reason for hiding this comment

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

Hm is this really an issue? I tried to repro, and to me, it seems like the bigger issue is most of the dashboard content is dynamic and on refreshes, I see the giant Ready to scale banner flash above the fold before it shifts downward which makes me think we'd be better off tweaking the other content on this page rather than the title?

Or maybe hold the banner until the data is loaded also? if i'm reading correctly, seems like the heading and most-content is contingent on the flow run count subscription but the banner isn't so i see the banner get shifted on load.

@pleek91
Copy link
Contributor Author

pleek91 commented Jun 18, 2024

Hm is this really an issue? I tried to repro, and to me, it seems like the bigger issue is most of the dashboard content is dynamic and on refreshes, I see the giant Ready to scale banner flash above the fold before it shifts downward which makes me think we'd be better off tweaking the other content on this page rather than the title?

Or maybe hold the banner until the data is loaded also? if i'm reading correctly, seems like the heading and most-content is contingent on the flow run count subscription but the banner isn't so i see the banner get shifted on load.

TBH maybe I "reproduced" the wrong thing. But I noticed that because the filters on the dashboard are v-ifd based on some async data. The header starts out as just the "Home" title (which is not as tall) and then when the filters load in the header grows in height causing the "Home" title to shift. Setting a min height fixed that so that "Home" didn't move.

But we could also await the data or use a loader on the whole page until the data is loaded. And that would also fix the problem I think.

@collincchoy
Copy link
Contributor

Hm is this really an issue? I tried to repro, and to me, it seems like the bigger issue is most of the dashboard content is dynamic and on refreshes, I see the giant Ready to scale banner flash above the fold before it shifts downward which makes me think we'd be better off tweaking the other content on this page rather than the title?
Or maybe hold the banner until the data is loaded also? if i'm reading correctly, seems like the heading and most-content is contingent on the flow run count subscription but the banner isn't so i see the banner get shifted on load.

TBH maybe I "reproduced" the wrong thing. But I noticed that because the filters on the dashboard are v-ifd based on some async data. The header starts out as just the "Home" title (which is not as tall) and then when the filters load in the header grows in height causing the "Home" title to shift. Setting a min height fixed that so that "Home" didn't move.

But we could also await the data or use a loader on the whole page until the data is loaded. And that would also fix the problem I think.

🤔 from what i can tell(by observation and also code), the actions actually always render in regardless of the flow run count subscription because v-if="!empty" returns true before the subscription is even finished executing since empty = subscription.response === 0 and undefined === 0 -> false and !false -> true
-😅 haha sorry for spelling it out. also for myself since it's not immediately obv that that's what happening.

separate thought, at small screens, the heading folds to vertical layout which min-h-11 is probably too small for.

@pleek91 pleek91 merged commit b93f38b into main Jun 18, 2024
6 checks passed
@pleek91 pleek91 deleted the fix-header-shift branch June 18, 2024 19:09
@aaazzam
Copy link
Collaborator

aaazzam commented Jun 18, 2024

TBH maybe I "reproduced" the wrong thing.

Nah that's what I had in mind when I made the issue -- thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for a bug in an existing feature ui Related to the Prefect web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: Layout shift in home component
3 participants