Fixes keyboard navigation in Ktable

[Pandaa007]

Description

I have taken the liberty to make several related changes in the KTable component:

• Refactored all of the handling in the handleKeyDown event into different internal methods so that the logic of the program is much clearer and maintainable in the future
• A new method was added to identify the focusable elements in a cell (rather than the older method of using hardcoded tags). Also, that method had an error that it focused on disabled elements as well, so I fixed that as well
• Corrected several bugs in the Tab + Shift navigation in case multiple focusable elements were present in a single cell. Earlier, if multiple focusable elements were in a cell, they were iterated in the left to right order, even if Tab + Shift was used, whereas the correct order should be right to left.
• Ensured that the order of elements in the Tab navigation and the Shift + Tab navigation were opposite.
• Fixed the bug with trapping on focus in the headers and first cells of the Ktable

Issue addressed

Fixes #883

Screenshots

LE.mp4

I used the following playground snippet to generate the sample page for testing (on which the demo video is recorded)

Playground

<template>

  <!--
    Playground: A private page for components development
    *******************
    Place a component you're working on here and see it
    live on http://localhost:4000/playground

    Please do not commit your local updates of this file.
  -->
  <div style="padding: 24px">
    <!-- 
      Example: Uncomment lines bellow, change something
      in lib/KBreadcrumbs.vue and see the updates reflected
      on this page
    -->
    <!-- <KBreadcrumbs
      :items="[
        { text: 'Global Digital Library', link: { path: '#' } },
        { text: 'English', link: { path: '#' } },
        { text: 'Level 2 ', link: { path: '#' } },
      ]"
    /> -->
    <!-- Play around with your component here: -->

    <KCheckbox label="First" />
    <KCheckbox label="Second" />

    <KTable
      :headers="headers"
      :rows="rows"
      caption="Table caption"
      sortable
    >
      <template #header="{ header }">
        <!-- <KCheckbox :label="header.label"/> -->
        <KButton>{{ header.label }}</KButton>
      </template>
      <template #cell="{ content, colIndex }">
        <span v-if="colIndex === 0">
          <!-- <KCheckbox :label="content"/> -->
          <KButton>{{ content }} 1 </KButton>
          <KButton disabled>{{ content }} 2 </KButton>
          <KButton>{{ content }} 3 </KButton>
        </span>
        <span v-else>{{ content }}</span>
      </template>
    </KTable>

    <KCheckbox label="Third" />

    <KTable
      :headers="headers"
      :rows="rows"
      caption="Table caption"
    />

    <KCheckbox label="Fourth" />
  </div>

</template>


<script>

  /*
    Playground is a Vue component too,
    so you can also use data, methods, etc.
    as usual if helpful
  */
  export default {
    name: 'Playground',
    data() {
      return {
        headers: [
          { label: 'Name', dataType: 'string', columnId: 'name' },
          { label: 'Age', dataType: 'number', columnId: 'age' },
          { label: 'City', dataType: 'string', columnId: 'city' },
        ],
        rows: [
          ['John Doe', 28, 'New York'],
          ['Jane Smith', 34, 'Los Angeles'],
          ['Emily Davis', 27, 'Philadelphia'],
        ],
      };
    },
    methods: {},
  };

</script> 

Changelog

• Description: Fixed bug in keyboard navigation in KTable
• Products impact: bugfix
• Addresses: [KTable]: Shift + Tab navigation is trapped #883
• Components: KTable
• Breaking: no
• Impacts a11y: yes
• Guidance: Fixes bug with keyboard navigation for consumers of KTable

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

[MisRob]

Hi @Pandaa007, thank you! I appreciate your critical and pro-active mindset, and it is true these improvements are needed. However I'd recommend to check on proposed changes that are out of scope beforehand in the issue comment with the team. Unfortunately, we won't be able to accept everything. Some of the extra issues you took on are already addressed and reviewed, and only wait for final QA here:

• Refactor handleKeydown Method for Improved Readability and Maintainability #804
• Improve the reverse navigation order of focusable elements within KTable cells #840

I'd recommend you only keep changes directly related to #883.

We're a bit behind on QA but I hope we can merge all pre-existing KTable work some time next week so perhaps it'd make sense for you to wait so later you don't need to deal with conflicts since as you can see in the PRs I referenced, there are many changes.

[Pandaa007]

Thanks @MisRob for the review. Totally my bad as I wasn't aware that these problems of refactoring and keyboard navigation were already being worked upon by other contributors, and thus felt that I might adress them here itself. As per your advice, I'd like to wait for them to be merged to the codebase first and then make my changes on top of them! Would really appreciate a tag whenever the same are merged and I can start working again on it. Thanks :)

[MisRob]

Thanks for your understanding @Pandaa007. I will make sure to let you know when it's time to rebase.

[MisRob]

Hi @Pandaa007, we're close to having everything ready for you. I will follow-up hopefully next week.

[MisRob]

Hi @Pandaa007, we've just merged

• Refactor handleKeydown Method for Improved Readability and Maintainability #804
• Improve the reverse navigation order of focusable elements within KTable cells #840

and KTable in develop is now the latest version, ready for more work.

I assume that #883 will still be valid but let' see.