Changed `ui.disable()` to modify opacity.
It previously tinted elements, which could have lackluster effects.
- Closes https://github.com/emilk/egui/issues/5850
- [x] I have followed the instructions in the PR template
New looks
(I say new but I tried to keep them as similar as possible to previous)
Light mode
Dark mode
Additional remarks
Since the opacity is being change, elements that are on-top of other elements now show some of the underneath element (this can be seen with the slider in the sent videos).
This has far-reaching consequences for the visual of existing egui apps other than just fixing the linked issue.
I think this change should be configurable; otherwise, significant considerations should be put into messing up existing visuals.
That's a fair viewpoint.
What other possible solutions to this issue do you recommend?
I tried modifying the fade_out_to_color method (as also suggested in the issue) to modify the alpha channel. However, this was not able to prevent the issue on light mode, even at the extremes of the alpha channel.
Relevant method: https://github.com/emilk/egui/blob/5d6aaa239b98ad20d365626dc664531b08b2537d/crates/egui/src/style.rs#L1055-L1060
Out of interest in which situations will this be an issue for (so I could potentially work on solving them)?
The problem is exactly what you mentioned in the Additional remarks sections.
I believe that a method of fading out (color or transparency) should be an option in the style. Maybe as an enum variant akin to
enum FadeOutMethod {
NonInteractiveWeakBgFill,
CustomColor(Color32),
DimTransparency(f32)
}
the exact shape of variants and naming should be discussed ofc
I agree with being able to override the fade out colour, but that does lean towards a more major change, as Visuals is a commonly used data structure.
Also adding more default colours increases the maintenance burden in the future. i'm not sure if that would be needed if the user can customise the fade out colour.
A possible option for setting a custom fade out colour would be to add a feild like override_fade: Option<Color32>, which can default to None.
Preview available at https://egui-pr-preview.github.io/pr/6765-defaultaffectsopacity Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.
Will it be a good idea to add an attribute to Visuals that is returned from fade_out_opacity?
Because currently the value is hard-coded, so it cannot be changed.
I have added the option for user-customization that i suggested & created a pull request against the branch in my fork for anyone to look at.
I can easily merge these changes into the branch in this egui pull request.
emilk, do you want me to merge the user-customisation code?
Yes, adding a Visuals::disabled_alpha: f32 that default to 0.5 sounds like a great idea
Yes, adding a
Visuals::disabled_alpha: f32that default to 0.5 sounds like a great idea
:+1: I have done so.
There is a problem with this approach: Visuals::weak_text_color uses Visuals::gray_out, but gray_out no longer makes the text gray; it makes it transparent. This may be fine, but we should probably rename gray_out, and we also need to update snapshot tests.
I will be able to work on this at the weekend
I've restored gray_out to its previous state, and created the new function disable that returns the colour with the alpha modified.
This allows for either to be used if modifying the alpha if undesirable.
An issue with the modify opacity approach is that the "colour picker" widget does not look very good when disabled on light mode.
Should i re-add the fade_out_to_color function, but annotated it as deprecated?
This will give time for users to stop using this function instead of having an instant compile error.
Should i re-add the
fade_out_to_colorfunction, but annotated it as deprecated? This will give time for users to stop using this function instead of having an instant compile error.
Yeah, why not.
You also need to update the snapshot images (again), because of a merge race (sorry!)
I think that you have to run the CI to generate the images again for me to pull them from the CI, apologies.
This should do it!