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

feat(Tag)!: remove deprecated props and update API #1862

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

booc0mtaco
Copy link
Contributor

@booc0mtaco booc0mtaco commented Feb 27, 2024

Summary:

  • remove deprecated value "yield" from variant list

We had both "warning" and "yield" as possible values for this component's variants, which was confusing. Going forward, we will start with just "warning", and remove this option from the component list. If using "yield", please consider using "warning" instead, or some other treatment.

  • change "icon" prop to be of type IconName to be consistent with all other components

This syncs up the headless Icon handling across all components, meaning that the names and possible values are determined by the mapped icons, and not any possible ReactNode instance. For consumers, replace any usages of <Icon /> with the string provided to this cmoponent. Any other nodes used will have to be removed.

  • tidy up usage of <Text /> and typography handling

simplify the handling of type in this component by moving to use preset instead of the more <font> like weight and size props, which will also be removed.

  • update tests, snapshots, and usages.

Test Plan:

  • update existing stories and tests
  • Wrote automated tests
  • CI tests / new tests are not applicable
  • Manually tested my changes, but I want to keep the details secret
  • Manually tested my changes, and here are the details:
    • Create an alpha publish and try out in edu-stack or traject as a sanity check if changes affect build or deploy, or are breaking, such as token changes, widely used component updates, hooks changes, and major dependency upgrades.

Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.65%. Comparing base (9ce8625) to head (e08d2eb).

Additional details and impacted files
@@           Coverage Diff           @@
##              v14    #1862   +/-   ##
=======================================
  Coverage   96.65%   96.65%           
=======================================
  Files         108      108           
  Lines        2151     2153    +2     
  Branches      574      575    +1     
=======================================
+ Hits         2079     2081    +2     
  Misses         70       70           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@booc0mtaco booc0mtaco requested a review from a team February 27, 2024 20:33
Copy link
Contributor

@ahuth ahuth left a comment

Choose a reason for hiding this comment

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

LGTM

@booc0mtaco
Copy link
Contributor Author

@jeremiah-clothier will try using size or distinct_size (referenced in the doc.s) to detect the number of commits to see how far back to reach for commitlint. Wondering if you tried either of those already, and they had some issue?

@jeremiah-clothier
Copy link
Contributor

@jeremiah-clothier will try using size or distinct_size (referenced in the doc.s) to detect the number of commits to see how far back to reach for commitlint. Wondering if you tried either of those already, and they had some issue?

Ohh maybe, I can look at this Thursday again. Rn the only workaround is to just not use any ' characters in commits. I'm pretty sure that's the issue

@booc0mtaco
Copy link
Contributor Author

@jeremiah-clothier will try using size or distinct_size (referenced in the doc.s) to detect the number of commits to see how far back to reach for commitlint. Wondering if you tried either of those already, and they had some issue?

Ohh maybe, I can look at this Thursday again. Rn the only workaround is to just not use any ' characters in commits. I'm pretty sure that's the issue

Yep, confirmed on two commits (one working and one not working). I'll try that out separate from this. if we merge a nice solution in next i can merge that so that it gets over here for this change

- remove deprecated value "yield" from variant list

We had both "warning" and "yield" as possible values for this
component's variants, which was confusing. Going forward, we will start
with just "warning", and remove this option from the component list. If
using "yield", please consider using "warning" instead, or some other
treatment.

- change "icon" prop to be of type `IconName` to be consistent with all
  other components

This syncs up the headless Icon handling across all components, meaning
that the names and possible values are determined by the mapped icons,
and not any possible ReactNode instance. For consumers, replace any
usages of `<Icon />` with the string provided to this cmoponent. Any
other nodes used will have to be removed.

- tidy up usage of `<Text />` and typography handling

simplify the handling of type in this component by moving to use
`preset` instead of the more `<font>` like weight and size props, which
will also be removed.

- update tests, snapshots, and usages.
@booc0mtaco booc0mtaco force-pushed the aholloway/EFI-1645/Tag branch from 5c42c3d to e08d2eb Compare February 28, 2024 16:39
@booc0mtaco booc0mtaco merged commit dbfbaa3 into v14 Feb 28, 2024
8 checks passed
@booc0mtaco booc0mtaco deleted the aholloway/EFI-1645/Tag branch February 28, 2024 16:47
@booc0mtaco booc0mtaco mentioned this pull request Mar 5, 2024
booc0mtaco added a commit that referenced this pull request Mar 5, 2024
## [14.0.0](v13.12.0...v14.0.0) (2024-03-05)

[Storybook](https://61313967cde49b003ae2a860-pjuqfmockf.chromatic.com/)

### ⚠ BREAKING CHANGES

* remove Grid and Section from exports (#1876)
* **Text:** remove deprecated props (#1873)
* **Heading:** remove deprecated props and refactor usages (#1872)
* **Link:** remove and reset deprecated props (#1871)
* **Select:** remove deprecated props (#1870)
* **Label:** remove deprecated props (#1868)
* **InputField:** remove unused prop requiredLabel (#1869)
* **InlineNotification:** remove deprecated props from component (#1867)
* **Card:** deprecate orientation prop (#1866)
* **Badge:** deprecate name prop on component (#1865)
* **Skeleton:** deprecate .Rect subcomponent mapping (#1864)
* **Tag:** remove deprecated props and update API (#1862)
* **Tooltip:** remove deprecated props (#1861)
* **Tabs:** combine Tab sub-component into Tabs
* remove deprecated components
* output a single, cjs build

### Features

* **Badge:** deprecate name prop on component ([#1865](#1865)) ([cff5d5e](cff5d5e))
* **Card:** deprecate orientation prop ([#1866](#1866)) ([bb160c1](bb160c1))
* **Heading:** remove deprecated props and refactor usages ([#1872](#1872)) ([a5d6f76](a5d6f76))
* **InlineNotification:** remove deprecated props from component ([#1867](#1867)) ([efc6b43](efc6b43))
* **InputField:** remove unused prop requiredLabel ([#1869](#1869)) ([f9d73dd](f9d73dd))
* **Label:** remove deprecated props ([#1868](#1868)) ([907cc39](907cc39))
* **Link:** remove and reset deprecated props ([#1871](#1871)) ([552945a](552945a))
* remove deprecated components ([2538aa6](2538aa6))
* remove Grid and Section from exports ([#1876](#1876)) ([a0ec79a](a0ec79a))
* **Select:** remove deprecated props ([#1870](#1870)) ([e0ad463](e0ad463))
* **Skeleton:** deprecate .Rect subcomponent mapping ([#1864](#1864)) ([9d3ffec](9d3ffec))
* **Tabs:** combine Tab sub-component into Tabs ([0479863](0479863))
* **Tag:** remove deprecated props and update API ([#1862](#1862)) ([dbfbaa3](dbfbaa3))
* **Text:** remove deprecated props ([#1873](#1873)) ([13fbc18](13fbc18))
* **Tooltip:** remove deprecated props ([#1861](#1861)) ([d444607](d444607))


### Bug Fixes

* output a single, cjs build ([523130e](523130e))
* remove unnecessary type definitions script ([be341c0](be341c0))
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.

3 participants