-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add colour customisation to layout editor #29936
base: master
Are you sure you want to change the base?
Conversation
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
#nullable disable |
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.
#nullable disable
is not allowed in any new code
@@ -29,6 +30,9 @@ public partial class ArgonSongProgress : SongProgress | |||
[SettingSource(typeof(SongProgressStrings), nameof(SongProgressStrings.ShowTime), nameof(SongProgressStrings.ShowTimeDescription))] | |||
public Bindable<bool> ShowTime { get; } = new BindableBool(true); | |||
|
|||
[SettingSource(typeof(SkinnableComponentStrings), nameof(SkinnableComponentStrings.Colour), nameof(SkinnableComponentStrings.ColourDescription))] | |||
public new BindableColour4 Colour { get; } = new BindableColour4(Colour4.White); |
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.
Not a fan of the shadowing here (the usage of new
) or the usage of base
underneath. I'd rename all of these to AccentColour
which gets rid of both.
@@ -20,11 +21,16 @@ public abstract partial class FontAdjustableSkinComponent : Container, ISerialis | |||
[SettingSource(typeof(SkinnableComponentStrings), nameof(SkinnableComponentStrings.Font), nameof(SkinnableComponentStrings.FontDescription))] | |||
public Bindable<Typeface> Font { get; } = new Bindable<Typeface>(Typeface.Torus); | |||
|
|||
[SettingSource(typeof(SkinnableComponentStrings), nameof(SkinnableComponentStrings.FontColour), nameof(SkinnableComponentStrings.FontColourDescription))] | |||
public BindableColour4 FontColour { get; } = new BindableColour4(Colour4.White); |
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.
Please rename this (and all associated members) to TextColour
. Fonts don't have colours.
Allows you to change colour for:
AdjustingColour.mp4
I also implemented this to
FontAdjustableSkinComponent
because it makes sense but I'm not too sure.I don't think there's design for button for adjusting colour, so I came up with my own.