Compacted scalars over different widths #16528
Conversation
|
You can't actually repro the bug with 1.5 million, as far as I can tell, so I will go and figure out how to do 1.5 trillion |
|
Also, is pictured: on mobile screen size the underlying problem doesn't occur at all, thanks to some responsive magic that existed beforehands |
|
Hey @howonlee, Not sure I'm the best person to review visualizations code. I guess @alxnddr has much more knowledge here than I do I'd be happy to review anyway, but could you please provide some extra context like what was the problem, how did it look before, and some steps to reproduce it? Would be perfect if there is a dedicated issue I can check Thank you! |
This stuff is all Tom and Paul anyways Underlying problem was that, if there were a lot of digits in a scalar being displayed in a box on a dashboard, it wouldn't compact properly if the box was too small. This was because the trigger for the compaction was based on overall box width alone, not anything to do with the content. This PR adds a consideration for the number of digits for the scalar. Issue to check is #12629 |
| @@ -33,6 +33,7 @@ function legacyScalarSettingsToFormatOptions(settings) { | |||
|
|
|||
| // used below to determine whether we show compact formatting | |||
| const COMPACT_MAX_WIDTH = 250; | |||
| const COMPACT_WIDTH_PER_DIGIT = 25; | |||
There was a problem hiding this comment.
It looks a bit hackish tbh even though works fine. What do you think about comparing the real width of the scalar element?
We have it in this._scalar
There was a problem hiding this comment.
I tried it and it starts flashing back and forth between the renders, giving it hysteresis would be pretty annoying I think
Problem: They don't compact when they should, sometimes #12629
Solution: Get the compaction to happen with respect to length of the full representation, not just to width of box