carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.87k stars 1.82k forks source link

fix(selectable-tile): don't require deprecated property "value" #18096

Closed wkeese closed 6 days ago

wkeese commented 1 week ago

Fixes #16977. Refs #13537, #13631.

Update PropTypes so "name" and "value" are reported as deprecated. (The descriptions already included @deprecated, but that's different.)

More importantly, stop marking "value" as required. Properties cannot be deprecated and required at the same time.

Changelog

Changed

Testing / Reviewing

Tested locally. Updated snapshot, and updated test file to stop using the deprecated parameters.

netlify[bot] commented 1 week ago

Deploy Preview for v11-carbon-web-components ready!

Name Link
Latest commit 02822cf636c5fba2394f329b3548057664559ad4
Latest deploy log https://app.netlify.com/sites/v11-carbon-web-components/deploys/673bdf403af0960008563834
Deploy Preview https://deploy-preview-18096--v11-carbon-web-components.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 1 week ago

Deploy Preview for carbon-elements ready!

Name Link
Latest commit 02822cf636c5fba2394f329b3548057664559ad4
Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/673bdf4074d695000841648d
Deploy Preview https://deploy-preview-18096--carbon-elements.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 1 week ago

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
Latest commit 02822cf636c5fba2394f329b3548057664559ad4
Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/673bdf40d4180200083a871d
Deploy Preview https://deploy-preview-18096--v11-carbon-react.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

riddhybansal commented 1 week ago

Hey this makes sense but failing test cases.

wkeese commented 1 week ago

I saw that, but isn't that an infrastructure problem, some issue with Jenkins etc.?

For me, the tests are all failing locally even on the main/ branch, with an error that

Cannot find module '@carbon/feature-flags' from 'packages/react/src/components/FeatureFlags/index.tsx'

That may be unrelated though.

wkeese commented 1 week ago

LGTM, just need to update the public api snapshot and push it up. yarn test -u at the root will do it

Ah thanks, I got it to work via running this from the root:

yarn install
yarn run build
yarn test -u
git commit -a --amend

(It's never clear to me when to run from root vs. when to run from packages/react.)

I force-pushed the latest.

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.21%. Comparing base (4fb87e7) to head (02822cf). Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #18096 +/- ## ======================================= Coverage 82.21% 82.21% ======================================= Files 404 404 Lines 14200 14200 Branches 4509 4513 +4 ======================================= Hits 11674 11674 + Misses 2364 2363 -1 - Partials 162 163 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

wkeese commented 1 week ago

I also updated the tests to remove the deprecated parameters.

Still getting minor visual diff failures, at https://percy.io/538fc19a/web/Carbon-Monorepo-/builds/37441342/changed/2039245365?browser=chrome&browser_ids=59&group_snapshots_by=similar_diff&subcategories=unreviewed%2Cchanges_requested&utm_source=github_status_public&viewLayout=side-by-side&viewMode=new&width=1366&widths=1366.

Any ideas how to fix those? I assume they are unrelated to my changes.

wkeese commented 6 days ago

The visual diffs are unrelated to this PR, you can see the same thing happening in #18106.

carbon-automation[bot] commented 5 days ago

Hey there! v11.71.0 was just released that references this issue/PR.