kolibri-design-system icon indicating copy to clipboard operation
kolibri-design-system copied to clipboard

Initial Implementation Of KTable Component

Open BabyElias opened this issue 1 year ago • 5 comments

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

  • [#727]
    • 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.

  • https://deploy-preview-727--kolibri-design-system.netlify.app/playground/
  • https://deploy-preview-727--kolibri-design-system.netlify.app/ktable/

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

Testing checklist

  • [x] Contributor has fully tested the PR manually
  • [ ] If there are any front-end changes, before/after screenshots are included
  • [x] Critical and brittle code paths are covered by unit tests
  • [x] 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 avatar Aug 13 '24 02:08 BabyElias

@BabyElias I linked the docs build preview in the pull request description because if I recall you don't have access to 'Checks' on the PR

MisRob avatar Aug 17 '24 16:08 MisRob

@BabyElias I think this would be also a good point to think about the table's public API. Even though some minor APIs may be subject to change, you've got to a great place of having the most important pieces functioning robustly, so it'd be great to reflect that in the public interface and documentation, especially as we're moving towards using the table in production 🥳 (long story short, you're doing such a good progress, so now there's some extra work 😁) So on a high-level, I'd like to offer a proposal that should help with clarity and reflect how the table behaves. Let's discuss together, feedback is welcome.

  • (1) Provide the simplest behavior by default. Gradually progress towards more advanced or less frequent use-cases configured by means of optional APIs
    • Why: A good component-design practice that makes using and documenting components easy and clear.
  • (2) Update the language around the table sorting modes to reflect the separation of concerns
    • There's no need to use "backend", "frontend", "server" and similar to define the table's sorting modes. By this I do not mean that these terms are prohibited :-) Later, when you document the integration with the externally sorted data, it'll be perfect to use the concrete example based on the server-side sorting use-case we work with as long as it's communicated as an example rather than the defining behavior of the table.
    • Why: The table doesn't and is not supposed to know about all these things. Its only concern is whether it should sort data internally or not. One example - sorted data doesn't need to necessarily come from the server, it could be the result of the client-side external custom sort function. For the table, it makes no difference. This is intended separation of concerns.

Put together:

Proposed API
Non-sortable <KTable>
Sortable with the default sorting enabled (simpler and the most frequent sorting use-case) <KTable sortable>
Sortable with the built-in sorting disabled (rather advanced and less frequent use-case) <KTable sortable disableBuiltInSorting> or <KTable sortable disableDefaultSorting> or similar

As part of this, language used on the documentation page would be updated accordingly. Instead of 'backend', 'frontend' etc., we'd use 'externally sorted', 'built-in', 'default' or similar.

MisRob avatar Aug 18 '24 08:08 MisRob

@radinamatic you can start testing on the playground before the Kolibri PR is finished. It has more use-cases, so it will be useful in any case.

MisRob avatar Aug 18 '24 08:08 MisRob

@radinamatic @LianaHarris360 @marcellamaki @AlexVelezLl Ready for your feedback! You're all welcome to review as much as you'd like or only chime in for some parts. There is also a Kolibri PR coming. I think I reviewed this PR carefully and would be nice to have more space for other reviewers so if someone would like to be the main assignee on the other PR, I would be grateful. I could still chime in briefly if needed. It would also help me time-wise since I will be re-reviewing this PR at some point.

@BabyElias Thanks for all the guidance and playground preparation, very helpful

Can't wait to see this live :) Thanks everybody.

MisRob avatar Aug 18 '24 08:08 MisRob

@radinamatic @LianaHarris360 @marcellamaki @AlexVelezLl

Some context for reviews and QA:

  • It's fine to improve existing places in code, and resolve blocking issues if we spot some in Kolibri, just note there's no nuanced RTL support yet
  • There's generally lots of features and modes we'll want to document, however at this stage, I asked @BabyElias to focus on documenting only high-level, with one example for each major table mode (third one, for externally sorted data, is about to come in another PR).

There will be some work on this by the end of GSOC if there is time left or there will be follow-up issues. There's already lots of great things done or in progress :)

MisRob avatar Aug 18 '24 08:08 MisRob

Everybody thanks for feedback and wonderful work with resolving it all @BabyElias. Talk tomorrow - we will log follow-ups and then open for final QA in Kolibri. If it shows no issues, will be ready for merge :partying_face:

MisRob avatar Aug 29 '24 20:08 MisRob

Fixed the failing test in KTable, the existing failing test comes from KDateRange.

BabyElias avatar Sep 01 '24 05:09 BabyElias

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

BabyElias avatar Sep 05 '24 17:09 BabyElias

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.

MisRob avatar Sep 05 '24 18:09 MisRob

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

MisRob avatar Sep 05 '24 19:09 MisRob