-
Notifications
You must be signed in to change notification settings - Fork 571
Turn ColorSelectorSliders into robust xaml + add hex color field to color sliders #6370
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
base: master
Are you sure you want to change the base?
Conversation
|
ready for review |
| /// A LineEdit control that displays in a monospace font. | ||
| /// Because the font is monospace, this allows you to resize the control based on the size | ||
| /// of a given "test string", using <see cref="MeasureText"/>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhm, that has nothing to do with it being monospace?
Also this API is too niche to be public.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Holy copy paste
|
tried my best to address the reviews |
|
|
||
| [GenerateTypedNameReferences] | ||
| [Access(typeof(ColorSelectorSliders))] | ||
| public sealed partial class AlphaSliderChannel : BoxContainer, ISliderChannel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be public
|
|
||
| [GenerateTypedNameReferences] | ||
| [Access(typeof(ColorSelectorSliders))] | ||
| public sealed partial class ColorSliderChannel : BoxContainer, ISliderChannel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be public
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to be clear:
- robust xaml can't generate controls that aren't public, right
- if yes, then in this case, we want these controls to be pure c#?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
robust xaml can't generate controls that aren't public, right
Yes it can?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh huh, i feel like i was getting errors last time i tried. i could give it a shot later...? are there examples in the code where this is done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there examples in the code where this is done?
All over the place in engine.
This PR cleans up the initialization code of
ColorSelectorSliders.csby turning it into a Robust XAML control, which, from what I can gather, is something we are aiming to do eventually anyway.It also adds a new hex color field to the control, allowing players to (for once!) actually copy-paste colors to/from the color sliders, which is IMO pretty important for QOL
color.mp4