Web: Remove hard coded coloring (mainly for discover related components)#25156
Web: Remove hard coded coloring (mainly for discover related components)#25156
Conversation
| const StyledFieldInput = styled(FieldInput)` | ||
| input { | ||
| border: 1px solid rgba(255, 255, 255, 0.1); | ||
| border: 1px solid ${props => props.theme.colors.text.placeholder}; |
There was a problem hiding this comment.
i think after @rudream changes, this will be called muted instead of placeholder right?
| 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}; |
There was a problem hiding this comment.
same here (note to self)
ravicious
left a comment
ravicious
left a comment
There was a problem hiding this comment.
I'm not sure if adding a new color variant to the theme is a good idea. If this follows some recent color scheme discussions that I'm not aware of then that's fine. But otherwise I feel like we should avoid adding new colors to the palette, especially if they don't seem to play well with the concept of custom themes: what if someone wants to have a theme that has no greys?
| 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; |
There was a problem hiding this comment.
Was this border supposed to be removed?
There was a problem hiding this comment.
yes, this border was unnecessary (early days of discover)
| light: '#FFFFFF', | ||
|
|
||
| grey: { | ||
| blueGrey: { |
There was a problem hiding this comment.
I renamed the theme colors (grey -> blueGrey and lightGrey -> grey) to its more appropriate form at the last two commits
"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.
| return state.data.isFixed | ||
| ? { | ||
| ...base, | ||
| backgroundColor: `${theme.colors.grey['600']} !important`, |
There was a problem hiding this comment.
Are we able to avoid adding colors.grey in the first place, like replace it here with another color? This seems to be the only place which uses it. 😶🌫️
|
closing: #25156 (comment) |
4 participants
I renamed the theme colors (grey -> blueGrey and lightGrey -> grey) to its more appropriate form at the last two commits