protofire / omen-exchange

Omen exchange
https://omen.eth.link/
GNU Affero General Public License v3.0
66 stars 77 forks source link

Ref: Update colours and text with new theme in Airdrop Modal #2185

Closed Frankaus closed 3 years ago

Frankaus commented 3 years ago

closes https://github.com/protofire/omen-exchange/issues/2116 closes https://github.com/protofire/omen-exchange/issues/2118 closes https://github.com/protofire/omen-exchange/issues/2117 closes #2119

We have redefined the Text styled component use in the new theme and applied it throughout the entire Airdrop modal together with the Guild Membership section

Note for review: Please let me know if these 2 colours on the numbers show correctly cause I couldn't test that.

Screenshot 2021-08-13 at 15 48 09
hexyls commented 3 years ago

@Frankaus just added a small example commit for how we might do this:

It looks like we mostly only need a custom prop for the color because we need to look up the color in our theme. But for the rest of the styling we can just go with the standard style prop:

bodyMedium(props: any) {
  const { style, ...rest } = props
  return <TextWrapper {...rest} style={{ fontSize: 14, fontWeight: 500, lineHeight: '18px', ...style }} />
},

we also expose the style prop and apply it last, just in case we want to override it elsewhere.

Frankaus commented 3 years ago

@Frankaus just added a small example commit for how we might do this:

It looks like we mostly only need a custom prop for the color because we need to look up the color in our theme. But for the rest of the styling we can just go with the standard style prop:

bodyMedium(props: any) {
  const { style, ...rest } = props
  return <TextWrapper {...rest} style={{ fontSize: 14, fontWeight: 500, lineHeight: '18px', ...style }} />
},

we also expose the style prop and apply it last, just in case we want to override it elsewhere.

Thank you so much for your help! Will test it now and move on:)

Mi-Lan commented 3 years ago

@Frankaus just added a small example commit for how we might do this:

It looks like we mostly only need a custom prop for the color because we need to look up the color in our theme. But for the rest of the styling we can just go with the standard style prop:

bodyMedium(props: any) {
  const { style, ...rest } = props
  return <TextWrapper {...rest} style={{ fontSize: 14, fontWeight: 500, lineHeight: '18px', ...style }} />
},

we also expose the style prop and apply it last, just in case we want to override it elsewhere.

Yeah I just looked into it! Thats how I would also go about it but I would maybe add addition this this so that we can have cleaner code. bodyMedium(props: any) { const { style, ...rest } = props return <TextWrapper {...rest} style={{ fontSize: 14, fontWeight: 500, lineHeight: '18px', ...style, ...rest }} /> }, Because with this below we can just write and also all non style would just be invalid and won't hurt the style `<TYPE.bodyMedium color={'blue'} onClick={() => { console.log('here') }} padding="22px 0 0 0" text-align="right" width={'100'}

Hersssse </TYPE.bodyMedium>`

Mi-Lan commented 3 years ago

which components to check out:

pimato commented 3 years ago

wrong green on the airdrop alert modal:

Bildschirmfoto 2021-08-17 um 13 58 58

should be green from our color palette

Frankaus commented 3 years ago

@pimato We need to change the way we call green red in our palette. Because with the new way we infer color into the textWrapper it seems that the machine takes a generic green red etc. Can you suggest a name for them pls? A simple green1 or greenOmen might solve it:)

pimato commented 3 years ago

@pimato We need to change the way we call green red in our palette. Because with the new way we infer color into the textWrapper it seems that the machine takes a generic green red etc. Can you suggest a name for them pls? A simple green1 or greenOmen might solve it:)

for which specific colors do we need this exactly?

Frankaus commented 3 years ago

@pimato We need to change the way we call green red in our palette. Because with the new way we infer color into the textWrapper it seems that the machine takes a generic green red etc. Can you suggest a name for them pls? A simple green1 or greenOmen might solve it:)

for which specific colors do we need this exactly?

for white green and red

pimato commented 3 years ago

@pimato We need to change the way we call green red in our palette. Because with the new way we infer color into the textWrapper it seems that the machine takes a generic green red etc. Can you suggest a name for them pls? A simple green1 or greenOmen might solve it:)

for which specific colors do we need this exactly?

for white green and red

just changed it from

white to snow red to alert and green to profit

hope this helps

Frankaus commented 3 years ago

Colours work well that way, nice catch. There was no need to edit the white because we use a standard #fff white hex code. In the event we wish to change that white I will edit inside the theme and make the changes else where as well. We are using the white value inside the button, global_style and react_tooltip_styles_override files so I that event I suggest to make the change in a separate PR. It's easy and quick to do

pimato commented 3 years ago

Date and Icon should be in one line:

Bildschirmfoto 2021-08-24 um 14 49 58