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: block cell nav events w internal focus queue active #6754

Merged
merged 4 commits into from
Aug 11, 2023

Conversation

scomea
Copy link
Collaborator

@scomea scomea commented Jun 6, 2023

📖 Description

Keyboard events emitted from a data grid cell when a cell's internal navigation is active can incorrectly move focus in the parent grid. Rather than rely on authors to intercept these events the cell component can prevent their propagation.

Additionally, this change adds

  • a simple 'defaultCellFocusTargetCallback` function that authors can use to move focus to the first element of a cell.
  • an example of a grid with cells with internal focus targets and internal nav queue
  • fixes code that detects the current active element to work within shadow doms

📑 Test Plan

Current tests all work

✅ Checklist

General

  • [ x] I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

@vnbaaij
Copy link
Contributor

vnbaaij commented Jun 6, 2023

Will this also target the archive-fast-elemene-v1 branch? Otherwise the fast-blazor repo can't use it...

@Ogglas
Copy link

Ogglas commented Jun 16, 2023

Any update on @vnbaaij comment? Is anything more needed before this can be merged?

@scomea
Copy link
Collaborator Author

scomea commented Jun 16, 2023

Any update on @vnbaaij comment? Is anything more needed before this can be merged?

Working with @vnbaaij on a Blazor solution that won't require porting something back, hopefully get some progress on that this weekend. JS authors can pretty easily work around this in their own implementations by catching the same events from their internal queues as this PR does, this change should make their lives easier though. Blazor authors are more constrained.

@Ogglas
Copy link

Ogglas commented Aug 7, 2023

Any update on this PR @scomea?

@scomea
Copy link
Collaborator Author

scomea commented Aug 7, 2023

Any update on this PR @scomea?

Just pinged the collaborators to get some movement on reviews.

@scomea scomea force-pushed the users/scomea/data-grid-cell-nav branch from 499101a to fd2f1dd Compare August 8, 2023 17:48
@chrisdholt chrisdholt merged commit a38e6b1 into master Aug 11, 2023
5 checks passed
@chrisdholt chrisdholt deleted the users/scomea/data-grid-cell-nav branch August 11, 2023 01:46
janechu pushed a commit that referenced this pull request Jun 10, 2024
* block cell nav events

* Change files

* better default focus fn

* update to lates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants