Migrate AvatarToken #19046
Migrate AvatarToken #19046
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
georgewrmarshall
added
the
team-design-system
There was a problem hiding this comment.
Hey @thebinij, this is looking good would you be able to add some before/after screencasts or screenshots to ensure no visual regressions before and after the migration? Thanks
| /** | ||
| * Test networks have special symbols that combine the network name and 'ETH' | ||
| * so that they are distinct from mainnet and other networks that use 'ETH'. | ||
| */ | ||
| export type TestNetworkCurrencySymbol = | ||
| (typeof TEST_NETWORK_TICKER_MAP)[keyof typeof TEST_NETWORK_TICKER_MAP]; | ||
| typeof TEST_NETWORK_TICKER_MAP[keyof typeof TEST_NETWORK_TICKER_MAP]; | ||
|
|
||
| /** | ||
| * An object containing preferences for an RPC definition |
There was a problem hiding this comment.
I don't think this file should have changes even if just formatting. Could you please revert thanks!
| export const ColorBackgroundColorAndBorderColor: ComponentStory< | ||
| typeof AvatarToken | ||
| > = (args) => ( |
There was a problem hiding this comment.
nit: does this need to be on a new line or was it automatically formatted that way by eslint?
| export const ColorBackgroundColorAndBorderColor: ComponentStory< | |
| typeof AvatarToken | |
| > = (args) => ( | |
| export const ColorBackgroundColorAndBorderColor: ComponentStory<typeof AvatarToken> = (args) => ( |
There was a problem hiding this comment.
It was automatically formatted by lint:eslint:fix
| * Possible values could be 'AvatarBaseSize.Xs' 16px, 'AvatarBaseSize.Sm' 24px, 'AvatarBaseSize.Md' 32px, 'AvatarBaseSize.Lg' 40px, 'AvatarBaseSize.Xl' 48px | ||
| * Defaults to AvatarBaseSize.Md | ||
| */ | ||
| size?: AvatarBaseSize; |
There was a problem hiding this comment.
Same question as AvatarNetwork is there a way we could allow for AvatarNetworkSize enum? Not sure if we want to mix enum names and components? Do you have any thoughts on this @brad-decker? If we update the engineer will have to use something like
<AvatarToken size={AvatarBaseSize.Lg} />
Which I'm not a huge fan of...
| @@ -290,7 +290,7 @@ export interface BoxProps extends React.HTMLAttributes<HTMLElement> { | |||
| * Use TEXT_ALIGN const from '../../../helpers/constants/design-system'; | |||
| * Accepts responsive props in the form of an array. | |||
| */ | |||
| textAlign?: BoxTextAlign | BoxTextAlignArray; | |||
| textAlign?: BoxTextAlign | BoxTextAlignArray | TextAlign; | |||
There was a problem hiding this comment.
Left a comment in another of your PRs addressing this
|
This is an old PR. New PR |
Explanation
Manual Testing Steps
Pre-merge author checklist
Pre-merge reviewer checklist
If further QA is required (e.g. new feature, complex testing steps, large refactor), add the
Extension QA Boardlabel.In this case, a QA Engineer approval will be be required.