fix(color-scale): use file size unit custom color when not using color scale #975
Conversation
| @@ -126,7 +126,7 @@ impl UiStyles { | |||
|
|
|||
| impl Size { | |||
| pub fn colourful(scale: ColorScaleOptions) -> Self { | |||
| if scale.size && scale.mode == ColorScaleMode::Fixed { | |||
| if scale.mode == ColorScaleMode::Fixed { | |||
There was a problem hiding this comment.
here I really don't understand why we were checking if scale.size was enabled (and if it was normal, why scale.age wasn't checked as well 🤔)
cc: @MartinFillon since you changed that condition last in #702.
There was a problem hiding this comment.
Well iirc it was causing another bug but idrc
|
This looks good to me 👍 Thanks for the PR! ping @cafkafk for CO review |
cafkafk
force-pushed
the
fix/size-coloring-tweaks-without-color-scale
branch
from
bc5d9b4
to
aac96d1
Compare
cafkafk
force-pushed
the
fix/size-coloring-tweaks-without-color-scale
branch
from
aac96d1
to
e52c367
Compare
There was a problem hiding this comment.
I removed the two update merge commits and instead rebased onto main.
It's not entirely clear to me whether or not changing if scale.size && scale.mode will introduce a regression for a previously fixed bug, @MartinFillon can you remember what this fixes specifically so we can ensure we don't break something?
|
Okay so took a deep dive the reason of this was to fix #684 |
|
clearly it seems that we have two behaviours conflicting with each other here. We need to decide on which one to keep. |
Hmm I don't see how they are conflicting 🤔 |
bew
changed the title
Fixes #682
After half a year I finally went down the rabbit hole and fixed this bug I reported 🙃
I tested different combination of options and the program seems to work correctly in all cases.
Let me know what you think, are there existing tests for this specific part of the renderer?
See how the file size unit is always using my custom colors when color scale is not applied on the file size:
And when using color scale for the file size:
