Skip to content

fix: Overwrite colors when merging text styles#841

Open
har7an wants to merge 4 commits into
mfontanini:masterfrom
har7an:fix/style-merge-overwrites-colors
Open

fix: Overwrite colors when merging text styles#841
har7an wants to merge 4 commits into
mfontanini:masterfrom
har7an:fix/style-merge-overwrites-colors

Conversation

@har7an

@har7an har7an commented Feb 9, 2026

Copy link
Copy Markdown

Rewrite TextStyle::merge to 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.

@mfontanini

Copy link
Copy Markdown
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 .rev() back and that's not it, would need to look into it. This is a PDF export of examples/demo.md:

image

@har7an

har7an commented Feb 16, 2026

Copy link
Copy Markdown
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!

har7an added 4 commits April 30, 2026 14:29
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.
which was previously broken due to the change in behavior when merging
text styles.
@har7an har7an force-pushed the fix/style-merge-overwrites-colors branch from 40b38d2 to 7c661be Compare April 30, 2026 13:48
@har7an

har7an commented Apr 30, 2026

Copy link
Copy Markdown
Author

Uhm, well, I didn't quite make it until the end of the week. Sorry about that!

I rediscovered my patches today and I think the issue you mentioned is fixed now. But I'm not quite sure how that part of the code works so I think you should give this a thorough manual test perhaps.

@har7an

har7an commented Apr 30, 2026

Copy link
Copy Markdown
Author

Or, of course, I'll happily do the testing for you if you tell me what I should do and what to look out for. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants