Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change color title of group container widget independently #2743 #2757

Merged
merged 5 commits into from
Aug 8, 2023

Conversation

Mathis-Hu
Copy link
Contributor

Change color title of group container widget independently.
see issue #2743.

If this pull request is merged, I'll implement the same behavior in DBWR.

@kasemir
Copy link
Collaborator

kasemir commented Aug 7, 2023

For the group widget, you add a line color to support separate foreground, background and line colors.
Since there's also a change in how they are used for texts vs. lines, GroupWidget#configureFromXML handles the mapping from older versions that don't have the line color to the new one. The GroupRepresentation should then simply use those 3 colors: Line always uses the line color, text uses foreground and background.

Instead, the group widget now has another old_colors flag to tell the presentation how to use them? Can't you remove that by simply setting foreground, background and line colors as desired in configureFromXML?

There are some nonfunctional changes in the formula autocomplete that are unrelated...

@Mathis-Hu
Copy link
Contributor Author

Mathis-Hu commented Aug 7, 2023

@kasemir I change the value of the colors directly in the GroupWidget#configureFromXML method as you asked (if I understand it well), I agree it's better that way. Thanks for your feedback. Do not hesitate to give more.

Sorry for the unrelated commit, I still have some struggle with GitHub for chosing some commits and not others.

Copy link
Collaborator

@kasemir kasemir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good: Widget has new 'line color', and representation simply uses line color for line, fore/back colors for text.

The conversion from older versions is handled in configureFromXML, which sets the new line color from the old foreground, for "TITLE" mode the background becomes the foreground etc.

It's possible that this conversion needs to be tweaked as we open a bunch of older displays and find that we missed something, but that would then be a follow-up update.

@shroffk shroffk merged commit ce2ac91 into ControlSystemStudio:master Aug 8, 2023
1 check 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.

3 participants