Adding border radius full to Box component
#16118
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
georgewrmarshall
changed the title
Box component
|
Probably will want to decide if the name will be |
| | borderWidth | 0, 1, 2, 4, 6, 8, 9, 10, 12 or array of numbers [1, 2] for responsive props | - | | ||
| | borderRadius | BORDER_RADIUS values or array of BORDER_RADIUS values for responsive props from [../ui/helpers/constants/design-system.js](https://github.com/MetaMask/metamask-extension/blob/develop/ui/helpers/constants/design-system.js) | - | | ||
| | borderStyle | BORDER_STYLE values or array of BORDER_STYLE values for responsive props from [../ui/helpers/constants/design-system.js](https://github.com/MetaMask/metamask-extension/blob/develop/ui/helpers/constants/design-system.js) | - | | ||
| | as | The polymorphic `as` prop allows you to change the root HTML element of the Box component. Defaults to 'div' | 'div' | | ||
|
|
There was a problem hiding this comment.
Realized that links to blobbing although probably quite helpful. Become rapidly out of date as we edit or update the design-system.js file so updating to link to the file in general
| @@ -9,8 +9,8 @@ import { | |||
| ALIGN_ITEMS, | |||
| JUSTIFY_CONTENT, | |||
| TEXT_ALIGN, | |||
| SIZES, | |||
There was a problem hiding this comment.
I'm just as amazed as you are the we only use SIZES for bordeRadius in the Box component 😮
| border-radius: 0.125rem; | ||
| border-radius: 2px; | ||
| } | ||
|
|
||
| &--rounded-sm { | ||
| border-radius: 0.25rem; | ||
| border-radius: 4px; | ||
| } | ||
|
|
||
| &--rounded-md { | ||
| border-radius: 0.375rem; | ||
| border-radius: 6px; | ||
| } | ||
|
|
||
| &--rounded-lg { | ||
| border-radius: 0.5rem; | ||
| border-radius: 8px; | ||
| } | ||
|
|
||
| &--rounded-xl { | ||
| border-radius: 0.75rem; | ||
| border-radius: 12px; | ||
| } | ||
|
|
||
| &--rounded-pill { | ||
| border-radius: 9999px; |
There was a problem hiding this comment.
Updating to pixels here for less cognitive load when making the calculation to rems. There isn't any benefit in using rems outside of font sizes.
georgewrmarshall
requested review from
darkwing,
garrettbear and
brad-decker
and removed request for
darkwing
Explanation
Currently, there is no border radius prop option for a "full" pill like border radius.
This is a problem because currently engineers have to add extra css to achieve this and there are many instances where this would be useful
In order to solve this problem, this pull request adds a
full(9999px) border radius option. It also separates the options from the mainSIZEconst in the design-system constants. It also updates tests, docs and stories.More Information
Some naming conventions from other libraries
Screenshots/Screencaps
After
Manual Testing Steps
Boxcomponent in search barPre-Merge Checklist
+ If there are functional changes:
I will update all the current uses of the SIZE const for borderRadius and deprecate SIZES.NONE in another PR