fix: Overwrite colors when merging text styles#841
Open
har7an wants to merge 3 commits intomfontanini:masterfrom
Open
fix: Overwrite colors when merging text styles#841har7an wants to merge 3 commits intomfontanini:masterfrom
har7an wants to merge 3 commits intomfontanini:masterfrom
Conversation
with another style element to ensure that the other style is always applied. Previously the code kept the current style if it was not undefined, effectively disabling color styling in some scenarios like modal dialogs.
for added clarity, since the previous name was confusing.
and does indeed overwrite the former colors with the latter.
Owner
|
Thanks for the fix! I wonder why this was this way initially. I do notice PDF export look broken now. I checked with the 
|
Author
|
Oh, right. I didn't even think of checking that. I'll try to reproduce this locally and see if I can come up with something until the end of the week. Thanks for the feedback! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rewrite
TextStyle::mergeto always overwrite the colors defined in self with those of other, if other defines background/foreground colors.Previously, the colors in self were kept (if previously defined, e.g. from default values), meaning that merging would only modify text size and attributes after colors had been set for the first time (from whatever source).
I noticed this while rewriting my theme config today, since the style configured for modal dialogs wasn't applied any longer. I remember an earlier release (I think 0.13, but I might be wrong) still applied the colors for modal dialogs correctly, but from 0.14 that feature was apparently broken.
As a fun side-effect of this change, the themes in parsed HTML don't have to be iterated in reverse any more.
I wrote a test to ensure this behavior is upheld in the future. I also tried a debug version on some of my own slides and all other theming aspects look correct to me.
User-facing changes
Users can now properly theme their modal dialogs again. Modal dialogs will no longer have the default colors applied for all elements of their UI.