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

🪟 🔧 Use CSS Custom Properties for colors #19344

Merged
merged 34 commits into from
Jan 10, 2023

Conversation

josephkmh
Copy link
Contributor

@josephkmh josephkmh commented Nov 11, 2022

What

Converts our SASS color variables from using hex codes to using CSS custom properties

How

  • Adds _customProperties.scss
  • Adjusts _colors.scss to use CSS Custom Properties instead of hex codes
  • Removes colors.$blue-transparent in this commit to avoid SVG <linear-gradient> choking on this syntax: rgba(--var-color-blue, .1)
  • Minor adjustments to CodeEditor to make it work with custom props
  • Removes transparent color variants in the Tooltip and TooltipTable components
  • Adds box-shadows to the theme/SCSS variables

There are a few changes in the appearance of the tooltips specifically. They are not longer slightly transparent, and are now instead fully opaque. Instead of the inline transparent values, I used colors from our theme instead. There may be very minor differences, but they should be barely noticeable:

Before:
image
image
image
image

After:
image
image
image
image

Recommended reading order

Top to bottom

@josephkmh josephkmh requested a review from a team as a code owner November 11, 2022 09:56
@octavia-squidington-iv octavia-squidington-iv added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Nov 11, 2022
@josephkmh josephkmh requested a review from edmundito November 16, 2022 20:43
@dizel852 dizel852 self-requested a review November 28, 2022 13:29
Copy link
Contributor

@dizel852 dizel852 left a comment

Choose a reason for hiding this comment

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

Code LGTM 👍
Also checked locally - seems like no issues

Copy link
Collaborator

@timroes timroes left a comment

Choose a reason for hiding this comment

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

rgba functions don't work with custom CSS properties and transparency. Discussed this offline, will need to adjust that before merging.

@josephkmh
Copy link
Contributor Author

rgba functions don't work with custom CSS properties and transparency. Discussed this offline, will need to adjust that before merging.

@timroes this has been addressed now!

@josephkmh josephkmh requested a review from timroes December 5, 2022 09:54
Comment on lines +29 to +30
rgba(255 106 77 / 80%) 30%,
rgba(97 94 255 / 80%) 80%,
Copy link
Contributor Author

@josephkmh josephkmh Dec 5, 2022

Choose a reason for hiding this comment

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

These two colors are kind of one-off colors that are part of a gradient, so I felt better defining them inline here than in a global variable.

Comment on lines -109 to -110
textColor90: rgba(colors.$dark-blue, 0.9);
darkBlue90: rgba(colors.$dark-blue, 0.9);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were unused legacy colors

@octavia-squidington-iv octavia-squidington-iv removed the area/platform issues related to the platform label Jan 9, 2023
Copy link
Collaborator

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested around the UI can't find any recession. Have not validated the connector builder, since Lake already checked this.

Copy link
Collaborator

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested around the UI can't find any recession. Have not validated the connector builder, since Lake already checked this.

@josephkmh josephkmh enabled auto-merge (squash) January 10, 2023 11:43
@josephkmh josephkmh merged commit 6033b11 into master Jan 10, 2023
@josephkmh josephkmh deleted the joey/css-custom-properties-colors branch January 10, 2023 14:25
jbfbell pushed a commit that referenced this pull request Jan 13, 2023
* use css custom properties for colors

* separate tokens by hue

* adjust stylelint

* fix code editor color transformations

* remove blue-transparent from color palette

* scope opacity to bg only

* rename to theme-light

* avoid transparent blue, use palette color instead

* bring back expanding hex values

* replace tooltip colors

* define overlay color in theme

* fix tooltip colors

* slightly darker link color

* remove unused SC colors

* add box shadow variables

* light tooltip table variant

* inline one-off colors for login

* add new colors to theme-light

* fix import name

* fix merge conflict with box shadows

* fix overlay bg color

Co-authored-by: lmossman <lake@airbyte.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants