-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Web: Remove hard coded coloring (mainly for discover related components) #25156
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,7 @@ export const PathInput = forwardRef< | |
|
|
||
| const StyledFieldInput = styled(FieldInput)` | ||
| input { | ||
| border: 1px solid rgba(255, 255, 255, 0.1); | ||
| border: 1px solid ${props => props.theme.colors.text.placeholder}; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think after @rudream changes, this will be called
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct |
||
| background: transparent; | ||
| color: white; | ||
| box-shadow: none; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,10 +21,9 @@ import { Box, Text } from 'design'; | |
|
|
||
| const Container = styled(Box)` | ||
| max-width: 1000px; | ||
| background-color: rgba(255, 255, 255, 0.05); | ||
| background-color: ${props => props.theme.colors.spotBackground[0]}; | ||
| padding: ${props => `${props.theme.space[3]}px`}; | ||
| border-radius: ${props => `${props.theme.space[2]}px`}; | ||
| border: 2px solid #2f3659; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this border supposed to be removed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, this border was unnecessary (early days of discover) |
||
| `; | ||
|
|
||
| interface CommandBoxProps { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,25 +25,25 @@ import { TextIcon } from 'teleport/Discover/Shared/Text'; | |
|
|
||
| const HintBoxContainer = styled(Box)` | ||
| max-width: 1000px; | ||
| background-color: rgba(255, 255, 255, 0.05); | ||
| background-color: ${props => props.theme.colors.spotBackground[0]}; | ||
| padding: ${props => `${props.theme.space[3]}px`}; | ||
| border-radius: ${props => `${props.theme.space[2]}px`}; | ||
| border: 2px solid ${props => props.theme.colors.warning}; ; | ||
| `; | ||
|
|
||
| export const WaitingInfo = styled(Box)` | ||
| max-width: 1000px; | ||
| background-color: rgba(255, 255, 255, 0.05); | ||
| background-color: ${props => props.theme.colors.spotBackground[0]}; | ||
| padding: ${props => `${props.theme.space[3]}px`}; | ||
| border-radius: ${props => `${props.theme.space[2]}px`}; | ||
| border: 2px solid #2f3659; | ||
| border: 2px solid ${props => props.theme.colors.text.placeholder}; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here (note to self) |
||
| display: flex; | ||
| align-items: center; | ||
| `; | ||
|
|
||
| export const SuccessInfo = styled(Box)` | ||
| max-width: 1000px; | ||
| background-color: rgba(255, 255, 255, 0.05); | ||
| background-color: ${props => props.theme.colors.spotBackground[0]}; | ||
| padding: ${props => `${props.theme.space[3]}px`}; | ||
| border-radius: ${props => `${props.theme.space[2]}px`}; | ||
| border: 2px solid ${props => props.theme.colors.success}; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,33 +18,39 @@ import React from 'react'; | |
| import { useTheme } from 'styled-components'; | ||
| import ReactSelectCreatable from 'react-select/creatable'; | ||
|
|
||
| import { StyledSelect } from 'shared/components/Select/Select'; | ||
|
|
||
| const styles = theme => ({ | ||
| multiValue: (base, state) => { | ||
| return state.data.isFixed ? { ...base, backgroundColor: 'gray' } : base; | ||
| return state.data.isFixed | ||
| ? { | ||
| ...base, | ||
| backgroundColor: `${theme.colors.grey['600']} !important`, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we able to avoid adding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
| : base; | ||
| }, | ||
| multiValueLabel: (base, state) => { | ||
| if (state.data.isFixed) { | ||
| return { ...base, color: theme.colors.text.primary, paddingRight: 6 }; | ||
| return { | ||
| ...base, | ||
| paddingRight: 6, | ||
| }; | ||
| } | ||
|
|
||
| if (state.isDisabled) { | ||
| return { ...base, paddingRight: 6 }; | ||
| } | ||
|
|
||
| return { ...base, color: theme.colors.text.primaryInverse }; | ||
| return { ...base }; | ||
| }, | ||
| multiValueRemove: (base, state) => { | ||
| return state.data.isFixed || state.isDisabled | ||
| ? { ...base, display: 'none' } | ||
| : { | ||
| ...base, | ||
| cursor: 'pointer', | ||
| color: theme.colors.text.primaryInverse, | ||
| }; | ||
| }, | ||
| menuList: base => { | ||
| return { ...base, color: theme.colors.text.primaryInverse }; | ||
| }, | ||
| }); | ||
|
|
||
| export type SelectCreatableProps = { | ||
|
|
@@ -77,18 +83,21 @@ export const SelectCreatable = ({ | |
| }: SelectCreatableProps) => { | ||
| const theme = useTheme(); | ||
| return ( | ||
| <ReactSelectCreatable | ||
| className="react-select" | ||
| components={{ | ||
| DropdownIndicator: null, | ||
| }} | ||
| styles={styles(theme)} | ||
| {...rest} | ||
| isMulti={isMulti} | ||
| isClearable={isClearable} | ||
| isDisabled={isDisabled} | ||
| autoFocus={autoFocus} | ||
| /> | ||
| <StyledSelect> | ||
| <ReactSelectCreatable | ||
| className="react-select-container" | ||
| classNamePrefix="react-select" | ||
| components={{ | ||
| DropdownIndicator: null, | ||
| }} | ||
| styles={styles(theme)} | ||
| {...rest} | ||
| isMulti={isMulti} | ||
| isClearable={isClearable} | ||
| isDisabled={isDisabled} | ||
| autoFocus={autoFocus} | ||
| /> | ||
| </StyledSelect> | ||
| ); | ||
| }; | ||
|
|
||
|
|
||
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.
"More appropriate" as in the new names describe the colors better or is there a doc that describes those colors as such? I couldn't find blue greys and greys in the application design system.
Overall I think we should probably use these greys as sparingly as possible since they don't seem to play well with the concept of custom themes. But I suppose this is something that we'll have to take care of once we get to actually allowing custom themes.