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.

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

[BD-46] feat: add missing tokens in the Button component #1924

Conversation

PKulkoRaccoonGang
Copy link
Contributor

@PKulkoRaccoonGang PKulkoRaccoonGang commented Jan 17, 2023

Description

  • ensure we have a separate design token for every CSS variable hook (example above) used in Button component
    image

Merge Checklist

  • If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
  • Does your change adhere to the documented style conventions?
  • Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
  • Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
  • Were your changes tested in the example app?
  • Is there adequate test coverage for your changes?
  • Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please add wittjeff and adamstankiewicz as reviewers on this PR.

Post-merge Checklist

  • Verify your changes were released to NPM at the expected version.
  • If you'd like, share your contribution in #show-and-tell.
  • 🎉 🙌 Celebrate! Thanks for your contribution.

@openedx-webhooks openedx-webhooks added the blended PR is managed through 2U's blended developmnt program label Jan 17, 2023
@openedx-webhooks
Copy link

Thanks for the pull request, @PKulkoRaccoonGang!

When this pull request is ready, tag your edX technical lead.

@codecov
Copy link

codecov bot commented Jan 17, 2023

Codecov Report

Base: 90.41% // Head: 90.41% // No change to project coverage 👍

Coverage data is based on head (77f7cf8) compared to base (5b7ecf2).
Patch has no changes to coverable lines.

❗ Current head 77f7cf8 differs from pull request most recent head b8b8035. Consider uploading reports for the commit b8b8035 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##            alpha    #1924   +/-   ##
=======================================
  Coverage   90.41%   90.41%           
=======================================
  Files         192      192           
  Lines        3120     3120           
  Branches      746      746           
=======================================
  Hits         2821     2821           
  Misses        283      283           
  Partials       16       16           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/add-missing-tokens-to-Button-component branch from 3a79945 to 202a8ca Compare January 25, 2023 15:52
@PKulkoRaccoonGang PKulkoRaccoonGang changed the title [BD-46] DRAFT: Add missing tokens to Button component [BD-46] feat: Add missing tokens to Button component Jan 25, 2023
@PKulkoRaccoonGang PKulkoRaccoonGang changed the title [BD-46] feat: Add missing tokens to Button component [BD-46] feat: add missing tokens to Button component Jan 25, 2023
@PKulkoRaccoonGang PKulkoRaccoonGang changed the title [BD-46] feat: add missing tokens to Button component [BD-46] feat: add missing tokens in the Button component Jan 25, 2023
@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/add-missing-tokens-to-Button-component branch 2 times, most recently from 83addb2 to ef64845 Compare January 30, 2023 01:55
@viktorrusakov viktorrusakov force-pushed the Peter_Kulko/add-missing-tokens-to-Button-component branch from 77f7cf8 to 57f3823 Compare February 10, 2023 09:35
--pgn-btn-focus-outline-color: #{$btn-brand-focus-outline-color};
--pgn-btn-focus-color: #{$btn-brand-focus-color};
--pgn-btn-focus-border-color: #{$btn-brand-focus-border-color};
--pgn-btn-focus-bg: #{$btn-brand-focus-bg};

box-shadow: $btn-box-shadow;
Copy link
Member

Choose a reason for hiding this comment

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

[curious] I see box-shadow: $btn-box-shadow; is on many of these .btn-* class names, but not all of them. Should it be? Even though I believe the variable is currently essentially set for having no box shadow, if it did have a box shadow, would it be applying equally across all the button variants or only a subset of them?

Copy link
Contributor Author

@PKulkoRaccoonGang PKulkoRaccoonGang Feb 15, 2023

Choose a reason for hiding this comment

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

Declaring the box-shadow: $btn-box-shadow; is necessary for some buttons on the documentation site. This only applies to the ability to change the value of this token in an Edxorg theme.

For example:

  • Openedx theme
    image

  • Edxorg theme
    image

@@ -44,34 +46,64 @@ $btn-primary-hover-border-color: var(--pgn-color-btn-hover-border-primary) !defa
$btn-primary-active-bg: var(--pgn-color-btn-active-bg-primary) !default;
$btn-primary-active-color: var(--pgn-color-btn-active-text-primary) !default;
$btn-primary-active-border-color: var(--pgn-color-btn-active-border-primary) !default;
$btn-primary-focus-focus-outline-color: var(--pgn-color-btn-focus-outline-primary) !default;
Copy link
Member

Choose a reason for hiding this comment

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

[nit/curious] Is this SCSS variable supposed to have the word focus back to back? Is this intended to be $btn-primary-focus-outline-color?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks
My bad

@viktorrusakov viktorrusakov force-pushed the Peter_Kulko/add-missing-tokens-to-Button-component branch from 57f3823 to 7b30687 Compare February 15, 2023 14:47
@adamstankiewicz adamstankiewicz merged commit f8c0506 into openedx:alpha Feb 15, 2023
@openedx-webhooks
Copy link

@PKulkoRaccoonGang 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-semantic-release
Copy link
Contributor

🎉 This PR is included in version 21.0.0-alpha.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

edx-requirements-bot added a commit that referenced this pull request Feb 17, 2023
* build: switch base to trying a rebase (#2047)

* build: pull from alpha before rebase master (#2049)

* build: git checkout instead of git switch (#2051)

* build: set base branch to alpha (#2053)

* feat: alpha release of design tokens (#1770)

* build: add alpha/beta to release CI workflow

* feat: design tokens with style-dictionary

BREAKING CHANGE: Introduces design tokens.

* [BD-46] resolve bootstrap conflicts with design tokens (#1800)

* feat: resolve some Bootstrap conflicts with design tokens

Co-authored-by: Peter Kulko <[email protected]>

* feat: add ability to generate CSS utility classes through design tokens

* docs: added background-color for color HEX values on CSS utility classes page

* fix: add missed border utilities

* docs: add ability to view CSS variables on CSS Utility Classes page

* chore: remove unused import

* feat: remove deprecated components

BREAKING CHANGE: all of the deprecated components were removed from Paragon, these include `CheckBoxGroup`, `Checkbox`, `Fieldset`, `Input`, `InputSelect`, `InputText`, `ListBox`, `Modal`, `RadioButtonGroup`, `StatusAlert`, `Table`, `TextArea`, `ValidationFormGroup`, `Button.Deprecated`, `Tabs.Deprecated`, `Dropdown.Deprecated`.

* chore: update tokens for Modal components

* docs: display computed CSS variables on components' pages

* refactor: update design tokens structure

* feat: add CLI interface for design tokens (#1846)

* fix: install dependencies in tokens module after installing Paragon

* fix: remove package.json files from tokens module

* refactor: update design tokens structure

* refactor: replace old tables with `DataTable` on documentation site (#1859)

* [BD-46] refactor: improve design tokens architecture (#1874)

* refactor: update design tokens naming and add missing ones

* feat: expose replace vars script to CLI

Co-authored-by: Viktor Rusakov <[email protected]>

* build: add workflow_dispatch to alpha branch to trigger release

* fix: trigger release

* fix: added custom title for CSS output files (#1985)

* feat: deleted value in reference design tokens (#1989)

* fix: ensure design tokens have a valid type attribute (#1992)

Adds `type` attribute to all design tokens per W3C design tokens spec.

* fix: add missing tokens for Button component (#1924)

* feat: add new tokens and cleanup after rebase

* feat: alpha release of design tokens (#1770)

* feat: design tokens with style-dictionary

BREAKING CHANGE: Introduces design tokens.

* [BD-46] resolve bootstrap conflicts with design tokens (#1800)

* feat: resolve some Bootstrap conflicts with design tokens

Co-authored-by: Peter Kulko <[email protected]>

* feat: add ability to generate CSS utility classes through design tokens

* docs: added background-color for color HEX values on CSS utility classes page

* fix: add missed border utilities

* docs: add ability to view CSS variables on CSS Utility Classes page

* chore: remove unused import

* feat: remove deprecated components

BREAKING CHANGE: all of the deprecated components were removed from Paragon, these include `CheckBoxGroup`, `Checkbox`, `Fieldset`, `Input`, `InputSelect`, `InputText`, `ListBox`, `Modal`, `RadioButtonGroup`, `StatusAlert`, `Table`, `TextArea`, `ValidationFormGroup`, `Button.Deprecated`, `Tabs.Deprecated`, `Dropdown.Deprecated`.

* chore: update tokens for Modal components

* refactor: update design tokens structure

* feat: add CLI interface for design tokens (#1846)

* fix: install dependencies in tokens module after installing Paragon

* fix: remove package.json files from tokens module

* refactor: update design tokens structure

* [BD-46] refactor: improve design tokens architecture (#1874)

* refactor: update design tokens naming and add missing ones

* feat: expose replace vars script to CLI

Co-authored-by: Viktor Rusakov <[email protected]>

* fix: trigger release

* feat: deleted value in reference design tokens (#1989)

* fix: ensure design tokens have a valid type attribute (#1992)

Adds `type` attribute to all design tokens per W3C design tokens spec.

* fix: add missing tokens for Button component (#1924)

* Revert "chore(release): sync from master to alpha (#2040)"

This reverts commit 14ba07c.

* feat: add new tokens and cleanup after rebase

---------

Co-authored-by: Adam Stankiewicz <[email protected]>
Co-authored-by: Viktor Rusakov <[email protected]>
Co-authored-by: Peter Kulko <[email protected]>
Co-authored-by: Viktor Rusakov <[email protected]>
monteri pushed a commit to raccoongang/paragon that referenced this pull request Aug 17, 2023
PKulkoRaccoonGang added a commit to raccoongang/paragon that referenced this pull request Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blended PR is managed through 2U's blended developmnt program released on @alpha
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants