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

Initial Implementation Of KTable Component #727

Merged
merged 104 commits into from
Sep 10, 2024

Conversation

BabyElias
Copy link
Contributor

@BabyElias BabyElias commented Aug 13, 2024

Description

This PR introduces the KTable component to KDS. The component is a flexible and customizable table component designed to handle a variety of data presentation needs within our application. It supports advanced features such as local sorting, sticky headers, keyboard navigation, and dynamic column resizing, making it suitable for both simple and complex data tables.

Issue addressed

328
330

Note to Reviewers :

  • The table supports both tab key and arrow key navigations to move between the cells.
  • The width of columns can be dynamically set by using the width and min-width props.
  • When viewport is shrinked, based on min-width/default width properties, scroll is enabled in the x & y-directions.
  • During scrolling in vertical direction, the column headers remain fixed and during scrolling in horizontal direction, the row headers remain fixed.
  • If content of a cell exceeds its size, text-wrapping takes place.
  • On mouse hover, the entire row over which mouse is hovered gets highlighted.
  • When navigating using keyboard (tab/arrow keys), the cell, the entire row containing the cell as well as the column header containing the cell is highlighted to maintain better readability and context.
  • The table supports another mode (not yet documented) - Sortable table with external sorting method. This external sorting can include cases where sorted data is fetched from backend, or use of custom sort functions with the table. An example of the same has been included in Playground page as Backend Sorted Table. In this example, clicking on any header will reverse the contents of the column - just an attempt to show the difference between external sorting and uselocalsorting.

Changelog

  • Description: Initial implementation of KTable component
  • Products impact: New Component
  • Addresses: #328
  • Components: KTable
  • Breaking: No
  • Impacts a11y: Yes
  • Guidance:

Steps to test

Sample tables have been added on the Playground Page for testing.

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical and brittle code paths are covered by unit tests
  • The change is described in the changelog section above

Reviewer guidance

  • Is the code clean and well-commented?
  • Are there tests for this change?
  • Are all UI components LTR and RTL compliant (if applicable)?
  • Add other things to check for here

After review

  • The changelog item has been pasted to the CHANGELOG.md

Comments

BabyElias and others added 30 commits May 30, 2024 21:18
KTable - [Trial] Added changeSort event handler
[KTable] Accessibility for Loading States
lib/__tests__/KTable.spec.js Outdated Show resolved Hide resolved
Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

@BabyElias I read through the latest implementation and haven't noticed anything blocking, so I think we're ready :)! Well done.

Just leaving a few cleanup notes before merge.

Also I can see the linter check is failing, would you run yarn lint-fix and commit?

@BabyElias
Copy link
Contributor Author

Ran yarn lint-fix, still shows failing lint errors.

docs/pages/ktable.vue Outdated Show resolved Hide resolved
@MisRob
Copy link
Member

MisRob commented Sep 5, 2024

Ran yarn lint-fix, still shows failing lint errors.

@BabyElias can you try to run it multiple times locally until it shows no error? I don't know why but I needed to run it twice. It will make some changes to docs/pages/ktable.vue that you will then need to commit. In case it doesn't work I can push it.

lib/KTable/index.vue Outdated Show resolved Hide resolved
@MisRob
Copy link
Member

MisRob commented Sep 5, 2024

Caught few typos in the latest updates, but other than that, looks good! Thank you :)

Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

Thank you, @BabyElias, let's release this :) Well done

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