Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Color cleanup number six #7050

Merged
merged 25 commits into from
Nov 8, 2021
Merged

Conversation

SimonBrandner
Copy link
Contributor

@SimonBrandner SimonBrandner commented Oct 28, 2021

Type: task


This should be reviewable commit-by-commit and doesn't need a design review as it shouldn't make any visible UI changes


This change is marked as an internal change (Task), so will not be included in the changelog.

Preview: https://618546ac487b9e0092a5aee8--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@github-actions github-actions bot added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Oct 28, 2021
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner SimonBrandner force-pushed the task/colors-6 branch 2 times, most recently from dc7b834 to f0719c3 Compare October 28, 2021 08:45
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
…older-fg-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner SimonBrandner marked this pull request as ready for review October 28, 2021 09:21
@SimonBrandner SimonBrandner requested a review from a team as a code owner October 28, 2021 09:21
@andybalaam
Copy link
Contributor

andybalaam commented Oct 28, 2021

This makes sense to me, so long as we don't later have themes that require different colours. I guess speculating about the future is a known-bad way to write software, so probably should be ignored...

By the way, my screenshot tests PRs (element-hq/element-web#19385 and #6938) would be a good way to test this doesn't actually change the visual appearance.

Does this affect custom themes? We are we removing the possibility of people customising button-primary-bg-color to be different from accent-color, right? Or am I misunderstanding custom themes?

Does this affect legacy themes? [Are there legacy themes that rely on the variables you are removing?]

@andybalaam
Copy link
Contributor

Does this affect legacy themes? [Are there legacy themes that rely on the variables you are removing?]

It looks like you are handling legacy themes in this change, so that looks ok.

I don't think I've got enough experience to give this a tick, but I like the look of it.

@SimonBrandner
Copy link
Contributor Author

SimonBrandner commented Oct 28, 2021

Does this affect custom themes? We are we removing the possibility of people customising button-primary-bg-color to be different from accent-color, right? Or am I misunderstanding custom themes?

Does this affect legacy themes? [Are there legacy themes that rely on the variables you are removing?]

The aim is to slowly remove colours that don't serve much purpose, currently, we have a lot of them and it makes working with the colour system quite painful. This removes some colours from custom and legacy themes though all colours should still be customizable just under the new color names from Figma Compound. In the end, we should end up with only the Figma Compound colours and the rest of the mess (colours which aren't in Compound at all) which we will also need to be cleaned up

@andybalaam
Copy link
Contributor

I approve, but I don't feel I have enough experience to tick.

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Copy link
Contributor

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

OK, I've thought about this a bit more, and I think I'm in a position to approve.

Great work, thanks!

@andybalaam andybalaam merged commit 07d9e93 into matrix-org:develop Nov 8, 2021
@SimonBrandner SimonBrandner deleted the task/colors-6 branch November 8, 2021 11:51
toger5 pushed a commit that referenced this pull request Nov 10, 2021
* Remove $voip-decline-color and $voip-accept-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Remove $dialog-background-bg-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $mention-user-pill-bg-color -> $warning-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $plinth-bg-color -> $secondary-accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $input-lighter-fg-color -> $input-darker-fg-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $input-valid-border-color -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $input-invalid-border-color -> $warning-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $button-bg-color -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $avatar-bg-color -> $background

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $preview-widget-fg-color -> $greyed-fg-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $settings-profile-button-fg-color -> $settings-profile-overlay-placeholder-fg-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $presence-online -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $pinned-unread-color -> $notice-primary-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $event-highlight-fg-color -> $warning-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $tab-label-active-bg-color -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $button-primary-bg-color -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $button-danger-bg-color -> $notice-primary-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $button-link-fg-color -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $togglesw-on-color -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $slider-selection-color -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $progressbar-fg-color -> $accent-color

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* $authpage-body-bg-color -> $background

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Fix merge issue

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Fix second merge issue

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants