Skip to content

Conversation

@lgriffee
Copy link
Contributor

@lgriffee lgriffee commented Nov 16, 2021

WHY are these changes introduced?

Part of solution for #4598

Cleaning up --p-button-font-weight and --p-badge-font-weight will allow our team to easily find and replace font-weight: 400 in the future when we define new tokens.

Removing up any unneeded global font-weight properties (ex. font-weight: inherit) will help keep our code clean and succinct.

WHAT is this pull request doing?

Replacing or removing the following font-weight values:

  • --p-button-font-weight
  • --p-badge-font-weight
  • inherit

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

🎩 checklist

@lgriffee lgriffee linked an issue Nov 16, 2021 that may be closed by this pull request
@github-actions
Copy link
Contributor

github-actions bot commented Nov 16, 2021

size-limit report

Path Size
cjs 165.96 KB (-0.02% 🔽)
esm 96.52 KB (-0.03% 🔽)
esnext 143.14 KB (+0.02% 🔺)
css 34.33 KB (+0.1% 🔺)

@lgriffee lgriffee changed the title [WIP] Remove font weight custom properties [WIP] Remove custom and unnecessary font weight properties Nov 16, 2021
border: border(transparent);
font-family: inherit;
font-size: inherit;
font-weight: inherit;
Copy link
Contributor Author

@lgriffee lgriffee Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this property can be safely removed since font-weight is being set to 400 via @include text-style-input on line 91

@lgriffee lgriffee changed the title [WIP] Remove custom and unnecessary font weight properties Remove custom and unnecessary font weight properties Nov 16, 2021
@lgriffee lgriffee marked this pull request as ready for review November 16, 2021 15:20
@lgriffee lgriffee self-assigned this Nov 16, 2021
Removed font-family and font-size since they are set in the mixin. Replaced font-family: inherit with explicit font-family() function for clarity.

Co-Authored-By: Kyle Durand <6844391+kyledurand@users.noreply.github.com>
Copy link
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. We just need to remove the values from:

buttonFontWeight: '500',

Copy link
Member

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the conflicts to clean up then :shipit:!

@lgriffee lgriffee merged commit 281f173 into main Nov 16, 2021
@lgriffee lgriffee deleted the 4598-remove-font-weight-custom-properties branch November 16, 2021 19:52
kyledurand added a commit that referenced this pull request Nov 18, 2021
kyledurand added a commit that referenced this pull request Nov 18, 2021
lgriffee pushed a commit that referenced this pull request Nov 19, 2021
@alex-page alex-page added this to the 2021Q4 milestone Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create simple font-weight tokens

4 participants