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: Convert Select from Main into a compound component #2309

Merged
merged 129 commits into from
Oct 20, 2023

Conversation

mannycarrera4
Copy link
Contributor

@mannycarrera4 mannycarrera4 commented Aug 3, 2023

Summary

Fixes: #1932

Release Category

Components

BREAKING CHANGES

  • We've converted Select in Main into a compound component. This component matches our pattern of providing access to lower-level elements and allows for more flexibility.
  • The spacing between menu and its target element will increase from 0px to 4px
  • We've also deprecated the Select in Preview. You may still consume this component but suggest migrating over to the one in Main.

Checklist

  • MDX documentation adheres to Canvas Kit's standard MDX template
  • Label ready for review has been added to PR

For the Reviewer

  • PR title is short and descriptive
  • PR summary describes the change (Fixes/Resolves linked correctly)
  • PR Release Notes describes additional information useful to call out in a release message or removed if not applicable
  • Breaking Changes provides useful information to upgrade to this code or removed if not applicable

Where Should the Reviewer Start?

Areas for Feedback? (optional)

  • Code
  • Documentation
  • Testing
  • Codemods

Testing Manually

Screenshots or GIFs (if applicable)

Thank You Gif (optional)

@cypress
Copy link

cypress bot commented Aug 3, 2023

1 flaky test on run #6348 ↗︎

0 965 2 0 Flakiness 1

Details:

Merge 3bfe4a6 into 10b7f1e...
Project: canvas-kit Commit: b07c96d1e1 ℹ️
Status: Passed Duration: 07:44 💡
Started: Oct 20, 2023 8:55 PM Ended: Oct 20, 2023 9:02 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 #2309 ↗︎

@myvuuu myvuuu added the 10.x label Aug 3, 2023
Comment on lines 83 to 85
if (matchIndex === -1) {
matchIndex = getIndexByStartString(0, keySofar.current, start);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

So after this step matchIndex will be always more than -1 or there is any possibility matchIndex will be 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if it's the first item

Copy link
Member

Choose a reason for hiding this comment

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

I think @RayRedGoose means to say "is there any possibility matchIndex will be -1"? A 0 means match found and it is the first item. This logic forces a wrap around from the current cursor, but it is possible a match is still not found.

@mannycarrera4 mannycarrera4 removed the ready for review Code is ready for review label Oct 20, 2023
@alanbsmith alanbsmith merged commit e60a421 into Workday:prerelease/major Oct 20, 2023
17 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.

6 participants