-
Notifications
You must be signed in to change notification settings - Fork 734
Infra/ revese color palettes in dark mode #1893
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
Conversation
src/style/colors.ts
Outdated
@@ -11,6 +11,7 @@ import Scheme, {Schemes, SchemeType} from './scheme'; | |||
|
|||
export class Colors { | |||
[key: string]: any; | |||
shouldSupportDarkMode = false; |
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.
Can you check if you can mark this class member as private
as private houldSupportDarkMode = false;
I think it will avoid having this member exposed to the user
src/style/colors.ts
Outdated
@@ -132,7 +134,8 @@ export class Colors { | |||
const colorKey = _.findKey(this, (_value, key) => this[key] === color); | |||
|
|||
if (colorKey) { | |||
const requiredColorKey = `${colorKey.slice(0, -2)}${tintKey}`; | |||
const key = this.shouldSupportDarkMode && Scheme.getSchemeType() === 'dark' ? 90 - Number(tintKey) : tintKey; |
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.
I'm not so sure about the 90 - Number(tintKey)
part?
Its purpose is to reverse the key to the dark mode key?
Since now we added keys like 1
and 5
it's possible we'll get a dark-mode key of 89
or 85
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.
Fixed
src/style/colors.ts
Outdated
@@ -143,6 +147,24 @@ export class Colors { | |||
return this.getTintedColorForDynamicHex(color, tintKey); | |||
} | |||
|
|||
getInvertedTintKey(tintKey: string | number) { |
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.
can I offer a code improvement (:
You can create an array of our keys ([1, 5, 10, 20, 30, ..., 80]
)
Then use array.indexOf
to detect the given key index, revert the index (as you did before) and return the inverted key..
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.
done :)
Description
Reverse color palettes in dark mode
WOAUILIB-2585
Changelog
Reverse color palettes in dark mode