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

frontend: use mui theming #2851

Merged
merged 28 commits into from
Feb 1, 2024
Merged

frontend: use mui theming #2851

merged 28 commits into from
Feb 1, 2024

Conversation

jecr
Copy link
Contributor

@jecr jecr commented Nov 27, 2023

This PR modifies the usage of themes, removing emotion's implementation to keep only a MUI theme with the Clutch design system's colors.

Testing Performed
manual

@jecr jecr marked this pull request as ready for review November 27, 2023 22:19
@jecr jecr requested a review from a team as a code owner November 27, 2023 22:19
@jecr jecr marked this pull request as draft November 30, 2023 19:02
@jecr jecr marked this pull request as ready for review November 30, 2023 19:55
jecr and others added 13 commits December 4, 2023 13:26
### Description
Replace hardcoded colors with theme palette references

### Testing Performed
Manual
Replace hardcoded colors with theme palette references for components in
AppLayout

Also updated the `landing` component

Testing Performed
manual, unit tests
Replace hardcoded colors with theme palette references for components in
input

Testing Performed
manual, unit tests

### Before (left) vs After (right)

**Checkbox Panel**
![Screenshot 2023-12-07 at 2 58 18 p
m](https://github.com/lyft/clutch/assets/25833665/e98a1672-77ce-4ad6-b934-2f7794524b8d)

**Checkbox**
![Screenshot 2023-12-07 at 2 58 45 p
m](https://github.com/lyft/clutch/assets/25833665/93dbf0c6-7854-4de9-bebe-c8a3e88c2b02)

**DateTimePicker**
![Screenshot 2023-12-07 at 2 59 26 p
m](https://github.com/lyft/clutch/assets/25833665/3a9ade30-d665-4406-b21a-01cc71828066)

**Form**
![Screenshot 2023-12-07 at 3 00 29 p
m](https://github.com/lyft/clutch/assets/25833665/f8c61df6-56e1-49f8-92f9-e33cc6e6a80d)

**MultiSelect**
![Screenshot 2023-12-07 at 3 00 57 p
m](https://github.com/lyft/clutch/assets/25833665/43a45a21-a6b8-42d2-a521-8a2b9afd5d25)

**RadioGroup**
![Screenshot 2023-12-07 at 3 01 29 p
m](https://github.com/lyft/clutch/assets/25833665/c8105c5b-e4e0-4c12-9653-047c49deb81e)

**Radio**
![Screenshot 2023-12-07 at 3 01 57 p
m](https://github.com/lyft/clutch/assets/25833665/184c618f-859e-4cd1-bb6a-2bfaf4845d47)

**Select**
![Screenshot 2023-12-07 at 3 02 20 p
m](https://github.com/lyft/clutch/assets/25833665/21abc090-05c1-4eb2-9904-a9c221cc6412)

**Switch**
![Screenshot 2023-12-07 at 3 02 45 p
m](https://github.com/lyft/clutch/assets/25833665/dc6875b2-126c-402f-8ef4-af310d92c791)

**TextField**
![Screenshot 2023-12-07 at 3 03 05 p
m](https://github.com/lyft/clutch/assets/25833665/34e1e6f4-e85a-4740-acc0-2cbdeebcc2da)

**TimePicker**
![Screenshot 2023-12-07 at 3 03 30 p
m](https://github.com/lyft/clutch/assets/25833665/08ba8853-6a15-41c7-a9dd-781352f6b05d)

**ToggleButtonGroup**
![Screenshot 2023-12-07 at 3 03 55 p
m](https://github.com/lyft/clutch/assets/25833665/9b1e9b22-a9f0-406e-9450-69af42538707)
Replace hardcoded colors with theme palette references for components in
NPS

Testing Performed
manual, unit tests

Before (top) vs After (bottom)

**Banner**
![Screenshot 2023-12-12 at 10 08 44 a
m](https://github.com/lyft/clutch/assets/25833665/bb54c974-d3b6-4c5b-9c31-dd4d5698e800)

**Header**
![Screenshot 2023-12-12 at 10 08 55 a
m](https://github.com/lyft/clutch/assets/25833665/cfef1f1f-07d5-4ddd-a2fd-1ca8925fa520)

**Wizard**
![Screenshot 2023-12-12 at 10 09 08 a
m](https://github.com/lyft/clutch/assets/25833665/b116b873-754b-4960-9929-8bcb15969e59)
This PR updates the following components to use the current theme
colors, additionally, updates matching colors across the whole app to
mantain consistency.

_before/after screenshots_

**Accordion**

![image](https://github.com/lyft/clutch/assets/5430603/44afebfd-bf9d-4648-a312-faf9547a5a62)


**Button**
Button Group (Primary)

![image](https://github.com/lyft/clutch/assets/5430603/c8d9314a-eb05-4841-bff2-b72aee11c634)

Button Group (Destructive)

![image](https://github.com/lyft/clutch/assets/5430603/941db941-5ad1-466b-b0a2-ab83c994211b)

Icon Button

![image](https://github.com/lyft/clutch/assets/5430603/9478c950-eb65-41b2-b93d-da65cb570417)


**Card**
Normal

![image](https://github.com/lyft/clutch/assets/5430603/cedde387-8c7f-4ae3-981b-e6fde37652b2)

Hover

![image](https://github.com/lyft/clutch/assets/5430603/d5a1dfaa-c122-4470-848c-14ae840e161a)


**Chip**

![image](https://github.com/lyft/clutch/assets/5430603/fb319611-b6c1-42c2-b761-d839f056687b)

![image](https://github.com/lyft/clutch/assets/5430603/a98809b8-5f60-4e09-bb50-df5a3f4a308b)

![image](https://github.com/lyft/clutch/assets/5430603/f7104917-63b8-4870-9fc0-af1f67229587)

![image](https://github.com/lyft/clutch/assets/5430603/9c96f6c7-a880-4b5a-ab96-fcd0d9337cfe)

![image](https://github.com/lyft/clutch/assets/5430603/7c1fda61-e64a-4718-8b1c-d62575fdca67)

![image](https://github.com/lyft/clutch/assets/5430603/ba167ac9-b7ce-4aa6-acf3-d7ac74f5422f)

![image](https://github.com/lyft/clutch/assets/5430603/0a773fb1-4125-427b-bab1-287614d77752)


**Confirmation**

![image](https://github.com/lyft/clutch/assets/5430603/8dd53187-0155-408a-a303-17d78682cc21)


**Tab**

![image](https://github.com/lyft/clutch/assets/5430603/9fbb0601-51ed-426b-8034-46256bb99f64)

![image](https://github.com/lyft/clutch/assets/5430603/17530d87-1e10-4f70-b442-f16cebf941ce)

![image](https://github.com/lyft/clutch/assets/5430603/00419a53-4cea-4266-aa47-b3c6f9d0534b)

![image](https://github.com/lyft/clutch/assets/5430603/fff90529-5d63-44c0-a36b-f0b794c5bad3)


_Note_
This PR mainly aims to update ungrouped components within
frontend/packages/core/src/, while updating all instances of widespread
colors as to avoid possible mismatches from working them on a directory
basis. Odd instances of hardcoded colors elsewhere will be addressed in
future PRs.

### Testing Performed
manual, unit testing
This PR updates the following components to use the current theme
colors, additionally, updates matching colors across the whole app to
mantain consistency.

_before/after screenshots_

### Dialog
<img width="1470" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/17f8f76a-02ca-480e-bae8-38d8bcde54ff">

<img width="1480" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/92483239-2054-45d2-a1a2-8bdd022cc72d">


### Icon
<img width="550" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/abab2f22-4469-40ed-b066-51f6f20fbfca">

<img width="547" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/41144f74-77ea-4055-a2cf-113e7f23e62f">

<img width="555" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/4edde15f-cbcd-4446-9e85-418ec2e00d99">

### Text
<img width="993" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/98f09ddd-ffde-4fb8-b573-becd69ee495a">


_Note_
This PR mainly aims to update ungrouped components within
frontend/packages/core/src/, while updating all instances of widespread
colors as to avoid possible mismatches from working them on a directory
basis. Odd instances of hardcoded colors elsewhere will be addressed in
future PRs.

### Testing Performed
manual, unit testing
Replace hardcoded colors with theme palette references for components in
worklows

Testing Performed
manual, unit tests

FYI for reviewers, this PR mainly aims to update components within
frontend/workflows, while updating all instances of widespread colors as
to avoid possible mismatches from working them on a directory basis. Odd
instances of hardcoded colors elsewhere will be addressed in future PRs.
This PR adds a brand color that matches the color used in the Clutch
logo, and updates the Not Found component to use this color from the
current theme.
Replaced string theme variation values (light, dark) with a shared
variable.
Fixed a miss-written theme usage in the project links component.

### Not found
No noticeable change was made:
before/after
<img width="1482" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/48663ff9-a3ec-45c1-84cb-106f3c418823">


### Testing Performed
manual, unit testing
Copy link

This PR has been marked as stale after 7 or more days of inactivity. Please have a maintainer add the on hold label if this PR should remain open. If there is no further activity or the on hold label is not added, this PR will be closed in 3 days.

@github-actions github-actions bot added the stale Issue hasn't had activity in awhile label Dec 27, 2023
@jecr jecr added on hold and removed stale Issue hasn't had activity in awhile labels Dec 27, 2023
lucechal14 and others added 5 commits January 5, 2024 18:59
frontend: update useTheme imports for workflows
Active state of the tab component was the same as the hover state, now
is darker.
Covers the following issues found in components in an ongoing theming
update:

App Layout - Header: Search autocomplete highlight color is too dark
before:

![image](https://github.com/lyft/clutch/assets/5430603/55fa7728-d32f-45de-9a76-a559daf78bc1)

after:

![image](https://github.com/lyft/clutch/assets/5430603/003046fb-21de-4e0a-8cd6-10b860b88e88)


Feedback: Darken text in feedback components for legibility
before:

![image](https://github.com/lyft/clutch/assets/5430603/74d5e3f1-3796-4588-bff7-17567249ccbd)

after:

![image](https://github.com/lyft/clutch/assets/5430603/ff9c4fe5-1d53-4b22-bd31-e9b4377cab1b)


Input - Switch: gray colors make it look disabled
before:

![image](https://github.com/lyft/clutch/assets/5430603/7b8ebc1a-ef11-4271-84bd-3693bfca11fe)
after: 

![image](https://github.com/lyft/clutch/assets/5430603/da243809-84d7-4efc-a477-117746a375ac)


Card: Landing card, icon character should be dark
before:

![image](https://github.com/lyft/clutch/assets/5430603/7e3d9f1b-aea9-4281-8044-bfcf06a4cabc)

after:

![image](https://github.com/lyft/clutch/assets/5430603/40f1b799-4261-4093-98ef-3c404026946b)


Accordion: Missing label (white text)
before:

![image](https://github.com/lyft/clutch/assets/5430603/546f1086-a547-4f43-9e52-4161a61e40ae)

after:

![image](https://github.com/lyft/clutch/assets/5430603/fc3a217f-b3fe-4125-872d-06b4a2707392)


Dialog: Pressed state should be a darker shade
normal state:

![image](https://github.com/lyft/clutch/assets/5430603/44063221-d5bc-4b71-90db-2807f2fe8a51)

before:

![image](https://github.com/lyft/clutch/assets/5430603/4cc9afa3-bd9d-48ed-b46b-5427ad6cd82e)

after:

![image](https://github.com/lyft/clutch/assets/5430603/5dc78d2b-44e3-4244-a638-e9e3902321e5)


Testing Performed
manual, unit tests
Project card close button was hite hich didn't make much contrast with
the background so it was changed to black.

Before:
<img width="372" alt="Screenshot 2024-01-12 at 10 50 17 a m"
src="https://github.com/lyft/clutch/assets/25833665/4bd73ea5-eeb9-4d18-a919-420ae8767563">

After:
<img width="365" alt="Screenshot 2024-01-12 at 10 49 53 a m"
src="https://github.com/lyft/clutch/assets/25833665/2e81642b-2641-41d0-a771-f9e906367c81">
Covers the following issues found in components in an ongoing theming
update:

Links became always underlined
before:
<img width="442" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/5fe90a1f-e96c-4769-a226-e18b75f5b0fa">

after:
<img width="443" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/87be5306-2f52-4421-9845-eeba9bbc6061">


Checkbox panel, label not visible due to text being white
before:
<img width="355" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/beb22993-0828-4df7-b1d8-bc3d66f14cdf">

after:
<img width="358" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/c214d80e-fe2b-414f-99f6-4c80df210672">


App Layout - Search: disable blue outline that appears on focus
before:
<img width="455" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/b9d73ca0-b8c0-48b2-99c0-fc7cf1d838bd">

after:
<img width="458" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/27fce5a5-b1c7-49be-9e5f-72751e564fa2">


### Testing Performed
manual, unit tests
jecr and others added 5 commits January 15, 2024 14:29
…eme.palette (#2899)

using theme.palette.contrastColor for white and black values
Palette usage instances were pointing to the secondary palette instead
of the error one.

before:
<img width="368" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/37bb7a83-c78a-4fca-a04f-93cbbccb20ca">

after:
<img width="358" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/a4d9591c-dafd-41c6-8910-f7752a54af03">

before:
<img width="971" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/b1d86efb-379a-4f65-940a-8bababf62996">

after:
<img width="978" alt="image"
src="https://github.com/lyft/clutch/assets/5430603/791cf77f-be12-4b24-9c5a-a1ef46c3cb4f">

Also noticed there are some redundant usages of style providers around
the app, so removed those.

### Testing Performed
manual, unit tests
When using the `theme` prop in `styled` components, it is not typed,
causing it to not have type checking.

This PR replaces all instances of `({ theme })` an adds its type as `({
theme }: { theme: Theme })`

### Testing Performed
manual, unit tests
@jecr jecr removed the on hold label Jan 22, 2024
Readability of tabs when hovered was reduced with the updated shade,
this pr adjusts it by making the using a lighter color shade for the
background.

before:

![image](https://github.com/lyft/clutch/assets/5430603/b1dd6f44-0bd4-4026-88d3-ca18b7e10cfa)

after:

![image](https://github.com/lyft/clutch/assets/5430603/95b89c27-0a1d-4f3b-abf1-cd2d799f9cc8)



### Testing Performed
manual, unit tests
Given the update from #2851 and since
it is a major update to clutch, this PR updates the current version of
clutch-sh/core (2.0.0-beta) to 3.0.0-beta to better represent the
changes.

Additionally updates the dependency versions in packages within the
current repo.

### Testing Performed
manual
@jecr jecr merged commit d84c283 into main Feb 1, 2024
9 checks passed
@jecr jecr deleted the use-mui-theming branch February 1, 2024 17:13
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