Skip to content

fix(menu): don't prevent setting the text color of an item - #513

Merged
ceccopierangiolieugenio merged 1 commit into
ceccopierangiolieugenio:mainfrom
slook:menu-set-text-color
Nov 8, 2025
Merged

fix(menu): don't prevent setting the text color of an item#513
ceccopierangiolieugenio merged 1 commit into
ceccopierangiolieugenio:mainfrom
slook:menu-set-text-color

Conversation

@slook

@slook slook commented Nov 7, 2025

Copy link
Copy Markdown
Contributor
  • Removed: a pointless check condition which meant the text could only be changed with a TTkString if it contained different text but not only a different color. This check is not required since setText() has to be called explicitly so setting a new text value is always wanted in any case.

For instance, you might want to make the existing text of a menu item bold or colored or blinking or whatever without actually changing what the string says, but the comparison check was done on the only the raw str value such as is returned by the TTkString constructor preventing such formatting changes from being made to the property.

@ceccopierangiolieugenio

ceccopierangiolieugenio commented Nov 8, 2025

Copy link
Copy Markdown
Owner

This is a tricky thing,
It seems that the menu.setText is never used in my tests and any style is and should be defined in the classStyle, but I think the TTkString needs a better or extended eq implementation, when I wrote it I considered 2 strings to be equal if the text is the same (also to avoid the overhead of comparing the list of colors), but I should introduce some kind of deep comparison to check when required that also the style is the same.
I will merge this, this method is not supposed to be abused and the extra signal trigered should not cause any issues or slowdown.
I am slowly trying to make everything typesafe so I will probably rework it a little when I will process this file.

Thanks

@ceccopierangiolieugenio
ceccopierangiolieugenio merged commit 370684e into ceccopierangiolieugenio:main Nov 8, 2025
11 checks passed
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