Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Layout foundations] Add the border width tokens to the Box component #7601

Closed
nayeobkim opened this issue Nov 1, 2022 · 3 comments
Closed

[Layout foundations] Add the border width tokens to the Box component #7601

nayeobkim opened this issue Nov 1, 2022 · 3 comments
Labels
no-issue-activity

Comments

@nayeobkim
Copy link
Contributor

Add the border width tokens

--p-border-width-1
--p-border-width-2
--p-border-width-3

to the Box component

Ensure that border width accepts colour tokens
@laurkim
Copy link
Contributor

laurkim commented Nov 1, 2022

Here are our current border color tokens:

  • border
  • border-on-dark
  • border-neutral-subdued
  • border-hovered
  • border-disabled
  • border-subdued
  • border-depressed
  • border-shadow
  • border-shadow-subdued
  • border-critical
  • border-critical-subdued
  • border-critical-disabled
  • border-warning
  • border-warning-subdued
  • border-highlight
  • border-highlight-subdued
  • border-success
  • border-success-subdued

@aveline aveline self-assigned this Nov 1, 2022
@aveline aveline mentioned this issue Nov 2, 2022
Merged
aveline added a commit that referenced this issue Nov 3, 2022
@aveline @kyledurand
[Box] Add border width support (#7621)
6e9edd3 
### WHY are these changes introduced?

Part of [#7601](#7601)

### WHAT is this pull request doing?

- Adds border with support only with this PR as further exploration is
needed to implement border color
- Sets the initial border width to `--p-border-width-1` which works
currently as all the existing shorthand border properties use that
width. Setting to `initial` doesn't work as the browser initial is
`medium` which overrides the shorthand width.

<details>
<summary>Playground code</summary>

```jsx
      <AlphaStack fullWidth>
        <Box
          paddingInlineEnd="10"
          paddingInlineStart="10"
          border="divider"
          borderWidth="1"
        >
          test
        </Box>
        <Box border="dark" borderWidth="3">
          test
        </Box>
        <Box border="dark" borderBlockStartWidth="3">
          test
        </Box>
        <Box border="dark" borderBlockEndWidth="3">
          test
        </Box>
        <Box border="dark" borderInlineStartWidth="3">
          test
        </Box>
        <Box border="dark" borderInlineEndWidth="3">
          test
        </Box>
        <Box border="dark" borderBlockEndWidth="2">
          test
        </Box>
        <Box
          border="divider"
          borderBlockEndColor="border"
          paddingInlineEnd="10"
          paddingInlineStart="10"
        >
          test
        </Box>
      </AlphaStack>
```

</details>

Co-authored-by: Kyle Durand <[email protected]>
@aveline aveline mentioned this issue Nov 3, 2022
Closed
@aveline aveline removed their assignment Nov 17, 2022
@aveline
Copy link
Contributor

aveline commented Nov 21, 2022

After discussing with @laurkim and @kyledurand we aligned on removing the shorthand border prop from box. Similar functionality will be supported by a new Divider component.

This will clear the way to add borderColor props on Box

@chazdean chazdean assigned chazdean and unassigned chazdean Nov 25, 2022
@github-actions
Copy link
Contributor

Hi! We noticed there hasn’t been activity on this issue in a while. After 30 days, it will close automatically.

If it’s still relevant, or you have updates, comment and let us know. And don’t worry, you can always re-open later if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
no-issue-activity
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@aveline @laurkim @chazdean @nayeobkim