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

fix: Revert spaceNumber values to pixel values and deprecate it #2345

Merged
merged 15 commits into from
Oct 19, 2023

Conversation

mannycarrera4
Copy link
Contributor

@mannycarrera4 mannycarrera4 commented Sep 20, 2023

Summary

Fixes: #2338

Release Category

Tokens

Release Note

We've deprecated spaceNumbers and encourage users to use our rem based space tokens.


@cypress
Copy link

cypress bot commented Sep 20, 2023

1 flaky test on run #6328 ↗︎

0 937 2 0 Flakiness 1

Details:

Merge a3eee0d into 9a4ff36...
Project: canvas-kit Commit: ec7b51ba01 ℹ️
Status: Passed Duration: 07:52 💡
Started: Oct 19, 2023 7:57 PM Ended: Oct 19, 2023 8:05 PM
Flakiness  cypress/integration/Autocomplete.spec.ts • 1 flaky test

View Output Video

Test Artifacts
... > when a value is entered > when down arrow key is pressed > when the user presses the enter key > when the use hits the "2" key > should change the filtered results Output Screenshots Video

Review all test suite changes for PR #2345 ↗︎

@mannycarrera4 mannycarrera4 marked this pull request as ready for review September 22, 2023 21:24
@mannycarrera4 mannycarrera4 added the ready for review Code is ready for review label Sep 22, 2023
@mannycarrera4
Copy link
Contributor Author

when looking at chromatic, in Major, the radio buttons in preview don't have the hover ring:
image

In this PR they are fixed
image

```tsx
// With `rem` based `space` tokens
{
padding: `calc(${space.xl} + 2px)`; // 42px
Copy link
Member

Choose a reason for hiding this comment

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

use PEMDAS

Copy link
Member

Choose a reason for hiding this comment

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

😉

@josh-bagwell josh-bagwell linked an issue Oct 19, 2023 that may be closed by this pull request
We've deprecated `Table` and `TableRow` as well as all of their exported members. Please consider using [`Table in Preview`](https://workday.github.io/canvas-kit/?path=/docs/preview-table--basic) instead.
**PR:** [#2344](https://github.com/Workday/canvas-kit/pull/2344)

We've deprecated `Table` and `TableRow` as well as all of their exported members. Please use [`Table`](https://workday.github.io/canvas-kit/?path=/docs/preview-table--basic) in Preview instead.
Copy link
Contributor

@josh-bagwell josh-bagwell Oct 19, 2023

Choose a reason for hiding this comment

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

Should we add a comment here on how the Table in preview is a full Table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you clarify? do you mean a compound component?

@josh-bagwell
Copy link
Contributor

Overall looks good! I just had some small comments.

@mannycarrera4 mannycarrera4 added automerge and removed ready for review Code is ready for review labels Oct 19, 2023
@alanbsmith alanbsmith merged commit fc8abda into Workday:prerelease/major Oct 19, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate spaceNumbers in v10 and revert values to pixel values
4 participants