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.

Sign up for GitHub

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

Jump to bottom

feat: combobox block #2725

Merged
merged 21 commits into from May 31, 2023
Merged

feat: combobox block #2725

merged 21 commits into from May 31, 2023

Conversation

AdamPawlinski
Copy link
Contributor

@AdamPawlinski AdamPawlinski commented May 19, 2023 •
edited

Related issue

Closes #655

Scope of work

  • ComboboxBasic block with dropdown select list and autocomplete logic for showing purposes
  • useTrapFocus - changes needed to make Combobox block work properly on keyboard events and added updateFocusalbeElements method to allow updating focusable elements manually,
  • fixes in Modal, SelectDropdown, MegaMenu and Search to adjust to useTrapFocus changes

Important: trapTabs property in useTrapFocus when switched to false doesn't allow to change focus outside the trap on pressing the tab key as it should be.

Screenshots of visual changes

Screenshot from 2023-05-26 13-37-30

Checklist

  • Self code-reviewed
  • Changes documented
  • Semantic HTML
  • SSR-friendly
  • Caching friendly
  • a11y for WCAG 2.0 AA
  • examples created
  • blocks created
  • cypress tests created

@changeset-bot
Copy link

changeset-bot bot commented May 26, 2023 •
edited

🦋 Changeset detected

Latest commit: 17407a7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@storefront-ui/react Minor
@storefront-ui/vue Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@AdamPawlinski AdamPawlinski changed the title 655 feat/combobox block feat: combobox block May 26, 2023
@AdamPawlinski AdamPawlinski marked this pull request as ready for review May 26, 2023 11:50
Szymon-dziewonski
Szymon-dziewonski reviewed May 29, 2023
View reviewed changes
Copy link
Contributor

@Szymon-dziewonski Szymon-dziewonski left a comment

Choose a reason for hiding this comment

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

Overall nice PR, i left few comments, all of them apply to vue and react but didnt want to create duplicates

FRSgit
FRSgit reviewed May 29, 2023
View reviewed changes
Copy link
Contributor

@FRSgit FRSgit left a comment •
edited

Choose a reason for hiding this comment

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

In Vue: when I focus input with tab an press arrowDown key I get an error like below:
image

@FRSgit
Copy link
Contributor

FRSgit commented May 29, 2023

In react when I choose any element and then tab into the input and hit arrow keys few times I get:
image

@AdamPawlinski
Copy link
Contributor Author

In Vue: when I focus input with tab an press arrowDown key I get an error like below: image

@FRSgit Now I'm thinking that there should be function in useTrapFocus that allows to update focusableElements manually, when changes in the dropdown happen, wdyt?

@FRSgit FRSgit self-requested a review May 30, 2023 13:58
@FRSgit FRSgit self-requested a review May 31, 2023 11:35
@sonarcloud
Copy link

sonarcloud bot commented May 31, 2023

[storefront-ui-vue] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented May 31, 2023

[storefront-ui-react] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@AdamPawlinski AdamPawlinski merged commit 05560b5 into v2-develop May 31, 2023
25 checks passed
@AdamPawlinski AdamPawlinski deleted the 655-feat/combobox-block branch May 31, 2023 14:40
This was referenced Jun 12, 2023
Merged
Closed
Merged
Closed
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Szymon-dziewonski Szymon-dziewonski Szymon-dziewonski left review comments

@FRSgit FRSgit FRSgit approved these changes

@dkacper dkacper Awaiting requested review from dkacper

@mattmaribojoc mattmaribojoc Awaiting requested review from mattmaribojoc mattmaribojoc is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@AdamPawlinski @FRSgit @Szymon-dziewonski