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

[KTable]: Refactoring the handleKeydown method in KTable #795

Open
4 tasks
BabyElias opened this issue Oct 4, 2024 · 6 comments
Open
4 tasks

[KTable]: Refactoring the handleKeydown method in KTable #795

BabyElias opened this issue Oct 4, 2024 · 6 comments
Assignees
Labels
Component: KTable help wanted Open source contributors welcome P2 - normal Priority: Nice to have

Comments

@BabyElias
Copy link
Contributor

🌱 Are you new to the codebase? Welcome! Please see the contributing guidelines.

Summary

In #727, we introduced a new component—KTable—which provides a flexible, accessible table layout with enhanced keyboard navigation capabilities. As part of this, the handleKeydown method is responsible for managing keyboard navigation between table cells, handling focus, and managing the arrow, tab, and other keys.
The current implementation of the handleKeydown method has grown more complex over time, making it harder to maintain and reason about. The goal of this issue is to refactor the method into a more readable and maintainable format, while preserving the functionality and accessibility of the keyboard navigation.

The Value Add

  • Improving overall code quality in the KTable component.
  • Ensuring that future modifications to the keyboard navigation behavior can be made more easily.
  • Improving overall code quality in the KTable component.

Possible Tradeoffs

Potential performance considerations: although the goal is to refactor for readability, care should be taken to avoid performance regressions, especially when handling key events frequently during table navigation.

Guidance

Please refer to this discussion for a better understanding of the implementation. In Summary,

  • Focus on breaking down the handleKeydown logic into smaller, modular methods that perform specific actions, such as handling different key events (i.e., arrow keys, tab, enter).
  • Avoid introducing unnecessary complexity; ensure the refactor simplifies the code.
  • Ensure that the refactor maintains the existing functionality, including:
    • Navigating between cells using arrow keys.
    • Handling focus management across rows and columns.
    • Supporting sticky headers and columns as described in the component behavior.
  • Include unit tests for the refactored code, especially around the key navigation and focus management aspects.

Acceptance Criteria

  • No regressions in existing keyboard navigation functionality.
  • Code is significantly more readable and easier to understand.
  • Unit tests are added or updated to cover the refactored method (if any)
  • No performance regressions due to the refactor.
@MisRob MisRob added help wanted Open source contributors welcome Component: KTable P2 - normal Priority: Nice to have labels Oct 4, 2024
@shivam-daksh
Copy link
Contributor

Hi @MisRob , May I look into this one?

@MisRob
Copy link
Member

MisRob commented Oct 7, 2024

Hi @shivam-daksh, thank you, I will assign you. Please have a look at a linked discussion to get a sense of direction and pay attention to acceptance criteria. KTable has many keyboard and screen reader features - I would recommend to spend some time understanding the current implementation in detail.

@shivam-daksh
Copy link
Contributor

@MisRob Sure I will. 😊

@MisRob
Copy link
Member

MisRob commented Oct 7, 2024

Lovely, thank you @shivam-daksh

@MisRob
Copy link
Member

MisRob commented Oct 15, 2024

@shivam-daksh as soon as you open a PR, would you please mention @BabyElias there as well? She's the author of the table and interested in joining the reviews. Thank you!

@MisRob
Copy link
Member

MisRob commented Dec 17, 2024

Hi @shivam-daksh, I wanted to mention that we will be closed from December 23 to January 5. It seems we will get to this QA rather next year, so hopefully then we can make progress toward merging all this wonderful work. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: KTable help wanted Open source contributors welcome P2 - normal Priority: Nice to have
Projects
None yet
Development

No branches or pull requests

3 participants