-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
🪟 🎉 New theme for Airbyte UI #18817
🪟 🎉 New theme for Airbyte UI #18817
Conversation
import en from "./locales/en.json"; | ||
import { Routing } from "./pages/routes"; | ||
import { WorkspaceServiceProvider } from "./services/workspaces/WorkspacesService"; | ||
import { theme } from "./theme"; | ||
|
||
const StyleProvider: React.FC<React.PropsWithChildren<unknown>> = ({ children }) => ( | ||
<ThemeProvider theme={theme}> | ||
<GlobalStyle /> |
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.
ℹ️ Removing the old GlobalStyle component using styled-components and replacing it by a global.scss
file that's loaded in index.tsx.
@use "./colors"; | ||
@use "./fonts"; | ||
|
||
html, |
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.
ℹ️ In the styled-component variant this still had a #__next
on it, which I assume (given we have 0 elements with that id) was a leftover from some very early versions of the app.
@@ -27,7 +27,7 @@ const run = keyframes` | |||
const Bar = styled.div` | |||
width: 100%; | |||
height: 49px; | |||
background: ${({ theme }) => theme.darkBeigeColor} url("/rectangle.svg"); | |||
background: #ffebd7 url("/rectangle.svg"); |
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.
ℹ️ Last place this beige is still used, but since we're planning to remove the onboarding page altogether soon, I just inlined the color for now here, instead of redesigning this progress bar.
@@ -74,10 +76,6 @@ $red-800: #bc0042; | |||
$red-900: #99003f; | |||
$red: $red-300; | |||
|
|||
$beige-50: #fef9f4; |
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.
ℹ️ No more beige color!!!
@@ -8,7 +8,7 @@ interface Props { | |||
} | |||
|
|||
const Box = styled.div` | |||
background: ${({ theme }) => theme.darkBeigeColor}; |
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.
@@ -20,7 +20,7 @@ const StepView = styled.div<{ | |||
min-width: ${({ lightMode }) => (lightMode ? "100px" : "auto")}; | |||
min-height: 28px; | |||
padding: 6px 14px; | |||
border-radius: 4px; | |||
border-radius: 28px; |
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.
@@ -84,7 +84,7 @@ const Th = styled.th<TableHeaderProps>` | |||
font-size: ${({ light }) => (light ? 11 : 10)}px; | |||
line-height: 12px; | |||
color: ${({ theme, highlighted }) => (highlighted ? theme.whiteColor : theme.lightTextColor)}; | |||
border-bottom: ${({ theme, light }) => (light ? "none" : ` 1px solid ${theme.backgroundColor}`)}; | |||
border-bottom: none; |
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.
ℹ️ This border was anyway barely visible (and not sure why we at all tried to have a beige border below the dark blue table header), so I removed it.
$blue-30: #f4f4ff; | ||
$blue-40: #f0efff; |
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.
ℹ️ Took from our color palette in Figma, those where still missing here.
@@ -16,8 +16,6 @@ | |||
margin-bottom: 24px; | |||
} | |||
|
|||
background: radial-gradient(35.57% 35.57% at 50% 50%, colors.$white 0%, colors.$white 55.87%, colors.$beige 100%); |
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.
ℹ️ On the new background it looks nicer just not having any radial gradient behind the error view.
cursor: pointer; | ||
border-radius: variables.$border-radius-md; | ||
color: colors.$grey-30; | ||
color: inherit; |
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.
ℹ️ Making sure this just inherits colors from the actual sidebar. Need to specify it explicitaly since browser have a default for color
on button that would prevent the inherit
otherwise.
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.
Nice, looks much cleaner 🙂 I tested locally and clicked around, couldn't find any visual problems.
Not approving for now, since we probably want a few more eyes on this!
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.
Code and new UI theme look great! Clicked around locally and didn't notice anything that looked off.
Only comment I have is that the Airbyte UI is now so bright that it would be cool to have a dark theme toggle built into the webapp. Maybe a future hackweek project!
What
Implements a new theme for the Airbyte UI as designed by Nico in this Figma: https://www.figma.com/file/liLQhcbpVHiEDW18kiXQe3/01_03_CONNECTIONS?node-id=1701%3A44970
It also cleans up relevant (S)CSS, SVG and components. See inline comments on this PR for more information.