report: prevent opportunity savings from wrapping #14619
Conversation
|
These wrapping bugs never die :) what happened since #14329? |
|
I guess the savings grew over time ;) |
report/assets/styles.css
Outdated
| @@ -874,6 +874,7 @@ | |||
| .lh-audit--load-opportunity .lh-audit__display-text { | |||
| text-align: right; | |||
| flex: 0 0 calc(4 * var(--report-font-size)); | |||
| white-space: nowrap; | |||
There was a problem hiding this comment.
The solution in #14329 was to increase the width instead of doing white-space: nowrap so the columns would stay aligned.
Could we just increase it again?
There was a problem hiding this comment.
The issue with that is, for those reports where savings are less digits (which I'm hoping is a good majority?), we'd leave a larger white space between sparkline and the savings. We applied 4 as the multiplier during that time on that basis, since it looked like a sweet spot. Yet a larger value still won't guarantee no breakage for a site that's worse-performing, or has an even higher savings in report.
I think an ideal case design would be a full refactor of opportunity tables with display: grid, where we could potentially minimize a column width to the maximum across all rows' content within that column. Seems theoretically possible, but I haven't tried yet.
This fix keeps the earlier sweet spot that works for majority, and decides to throw sparklines off a bit for offenders, but does guarantees a no-wrapping outcome for any value.
... or, should we refactor with display: grid? Overkill? wdyt?
PS: I see that you had commented on an earlier issue about white-space: nowrap.
There was a problem hiding this comment.
... or, should we refactor with display: flex? Overkill? wdyt?
I think the grid approach is tempting but probably not worth the work. I think that having some risk of word wrap is better than having some risk of the sparklines not lining up.
There was a problem hiding this comment.
just to make sure I'm reading you right — you'd prefer the wrapped column with extra height, than sparklines misaligned? I.e., screenshot 1 is better than screenshot 2?
There was a problem hiding this comment.
Yes, otherwise the sparklines could appear longer than they actually are. I still think we should try expanding the number's area though.
I confirmed this exact problem happens even for The root cause here is: We come up with a fixed value for the flex-basis of this item, such that all these independent flex containers will line up on the end of the sparkline. |
report/assets/styles.css
Outdated
| @@ -874,6 +874,7 @@ | |||
| .lh-audit--load-opportunity .lh-audit__display-text { | |||
| text-align: right; | |||
| flex: 0 0 calc(4 * var(--report-font-size)); | |||
| white-space: nowrap; | |||
There was a problem hiding this comment.
If you look closely, the "after" sparkline no longer lines up at the end. This is the same result as just deleting the flex: property above, but defeats the purpose of the rule: these sparklines need to line up across these isolated flex containers.
There was a problem hiding this comment.
Yeah that was intentional, in the original PR. Also I think if we use tables for rendering tables, this issue might not have occurred.
I've updated the PR with 5 and removed nowrap.
(EDIT: these images do not reflect the changes made in the PR)
It will maintain right alignment of sparklines as before, until the savings get too big, then it will prioritize not wrapping.
Before:
After:
