Calcite-components: Question: border-radius - Seems like we should use Tailwind or add it to our config

Created on 10 Jun 2021  ·  14Comments  ·  Source: Esri/calcite-components

Question

Right now, there's a CSS var for border radius.
Seems like we should either use Tailwind's default border-radius (which is 0.25rem/4px) or store our custom one in the config, yeah?

Helpful Details

Tailwind's border radius: https://tailwindcss.com/docs/border-radius

4 - verified question

Most helpful comment

@caripizza I think default is 4px.

I'm not so much proposing a new value. :)
I just noticed that we're defining a 3px radius in our global.scss and wondering if we should't be using default.

All 14 comments

cc @bstifle

as long as we have 4px border radius im a happy boi

Our current config includes the following values (mostly defaults):

    borderRadius: {
      none: "0",
      sm: "0.125rem",
      default: "0.25rem",
      md: "0.375rem",
      lg: "0.5rem",
      xl: "0.75rem",
      "2xl": "1rem",
      "3xl": "1.5rem",
      full: "9999px",
      half: "50%"
    },

What new values are you proposing?

@caripizza I think default is 4px.

I'm not so much proposing a new value. :)
I just noticed that we're defining a 3px radius in our global.scss and wondering if we should't be using default.

@asangma Got it, it looks like the --calcite-border-radius var was added long ago in pr #235, so it's a super old value. As far as I know, we can refactor it out at this point and go with your default 4px suggestion.

Do you want to take a crack at that work, as a part of this issue? (Thinking we can also remove that px 1px borderWidth addition in the config, since that's already included in Tailwind's defaults too.)

@caripizza I might be able to get to it this sprint or next. It'll include refactoring a couple stylesheets I think.

@asangma what if we do this in global.scss instead? (Might require less stylesheet changes.)

--calcite-border-radius: theme("borderRadius.default");

@julio8a can you add this one to your alert refactor? I think we should keep using this --calcite-border-radius global CSS var if we can (just needs the update from my above comment).

Self-assigning this one per conversation with Julio

@asangma I jumped the gun on this one, calcite-alert really wants that 3px border-radius. I was hoping to refactor it to only have a 2px top border width, but now that its corners are rounded (pr #2339), it shows a wonky curved top border with anything other than the 3px border-radius. I'm leaving this value in place for now and we can revisit down the road if needed.

@caripizza the only border radius in CC should be 4px :)

let's make default 4px.

Got it @bstifle

Assigning to @asangma for verification.

Verified in master

Was this page helpful?
0 / 5 - 0 ratings