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

[Accordion] Remove extraneous code with home/end/arrow keys #6081

Closed
macandcheese opened this issue Dec 16, 2022 · 4 comments
Closed

[Accordion] Remove extraneous code with home/end/arrow keys #6081

macandcheese opened this issue Dec 16, 2022 · 4 comments
Assignees
@geospatialem
Labels
4 - verified Issues that have been released and confirmed resolved. Calcite (design) Issues logged by Calcite designers. refactor Issues tied to code that needs to be significantly reworked.
Milestone
2023 January Prio...

Comments

@macandcheese
Copy link
Contributor

macandcheese commented Dec 16, 2022 •
edited
Loading

Actual Behavior

I am working on making some of our focus helpers into a general utility : #6075 (comment)

While doing so, I discovered our accordion keyboard navigation (besides tab / shift + tab) stopped working at some point: https://codepen.io/mac_and_cheese/pen/LYBPyPr?editors=1000

Expected Behavior

After discussion, this is the intentional behavior, the stale code was never removed - this issue has been repurposed to remove that code.
@macandcheese macandcheese added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Dec 16, 2022
@github-actions github-actions bot added the Calcite (design) Issues logged by Calcite designers. label Dec 16, 2022
@macandcheese macandcheese self-assigned this Dec 16, 2022
@macandcheese
Copy link
Contributor Author

After discussing with @geospatialem - this may be intentional / expected. But we still have a bunch of code in the components that supposedly is handling this, so we can remove if this is indeed desired behavior.

@geospatialem
Copy link
Member

We're going to proceed with leaving the Home / End / arrow keys as-is, instead repurposing this issue to remove the extraneous code to support the pattern.

@geospatialem geospatialem added this to the 2023 January Priorities milestone Dec 16, 2022
@geospatialem geospatialem changed the title [Accordion] Navigation with home/end/arrow keys no longer works [Accordion] Remove extraneous code with home/end/arrow keys Dec 16, 2022
@geospatialem geospatialem added refactor Issues tied to code that needs to be significantly reworked. 1 - assigned Issues that are assigned to a sprint and a team member. and removed bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Dec 16, 2022
@macandcheese macandcheese mentioned this issue Dec 16, 2022
Merged
macandcheese added a commit that referenced this issue Dec 17, 2022
@macandcheese
refactor(accordion, dropdown, stepper, tabs): Updates keyboard handli…
4b4d8b0 
…ng (#6086)

**Related Issue:** #6081
[#6803](#6083)

## Summary
As suggested by @alisonailea [in this
comment](#6075 (comment)),
extracts some common keyboard navigation focus helpers into a shared
utility.

Along the way discovered a few things that are fixed alongside the
refactors:

Accordion:
- Cleanup: The keyboard expected navigation changed at some point, which
I confirmed with @geospatialem, but there was a lot of related stale
code that is removed.

Dropdown:
- Fix: Previously, tab / shift tab would skip elements resulting in
navigating to every other item.
- Fix: Previously, there was inconsistent close behavior when using tab
/ shift tab at the first / last item.
- Fix: Previously, when `accordion-item` had a populated `href`
property, there was a "double focus" issue.
- Cleanup: Uses the new utility to handle keyboard navigation / focus


Stepper / Tabs:
- Cleanup: Uses the new utility to handle keyboard navigation / focus


All existing tests are passing, but we may want some extra passes of
manual testing, especially in the dropdown case, since there were so
many weird bugs previously.
@macandcheese macandcheese added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Dec 17, 2022
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Dec 29, 2022
@geospatialem
Copy link
Member

Verified on master and in the accordion component in beta.99.

The dropdown, stepper, and tabs components added enhancement utility is also working great in beta.99.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

Labels
4 - verified Issues that have been released and confirmed resolved. Calcite (design) Issues logged by Calcite designers. refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Milestone
2023 January Priorities
Development

No branches or pull requests
2 participants
@macandcheese @geospatialem