kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

Replace TabsWithOverflow with KListWithOverflow

Open AllanOXDi opened this issue 1 year ago • 4 comments

Summary

  • Migrate use of TabsWithOverflow to the KListWithOverflow component to be released in KDS
  • closes #12011

References

#12011

Reviewer guidance

Do the changes look good to you?

Testing checklist

  • [ ] Contributor has fully tested the PR manually
  • [ ] If there are any front-end changes, before/after screenshots are included
  • [ ] Critical user journeys are covered by Gherkin stories
  • [ ] Critical and brittle code paths are covered by unit tests

PR process

  • [ ] PR has the correct target branch and milestone
  • [ ] PR has 'needs review' or 'work-in-progress' label
  • [ ] If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • [ ] If this is an important user-facing change, PR or related issue has a 'changelog' label
  • [ ] If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

AllanOXDi avatar Mar 29 '24 14:03 AllanOXDi

is there any reason we're not using KDS >= v4.0.0 on develop? If so then this is as good a place as any to make that update eh?

@nucleogenesis KDS v4 is the release containing rebranding and it will be introduced in this PR https://github.com/learningequality/kolibri/pull/11911, at first to the release branch (and from there merged to the develop I assume)

KListWithOverflow is also available in KDS v3.1.1 so it can be installed into branches that have no branding changes yet. So I think you could just install 3.1.1 in this PR.

MisRob avatar Apr 02 '24 08:04 MisRob

@AllanOXDi @nucleogenesis I'm not familiar with the details of work, just noticed that currently it seems we're moving away from KTabsList to KListWithOverflow + plus some custom logic. Just an alert that KListWithOverflow can't replace KTabsList by no means since KTabsList contains tons of logic related to a11y specific to tabbed interfaces whereas KListWithOverflow is much more general. My recommendation would be to return it and also invite @radinamatic as soon as work here is done to give final confirm.

Relatedly, KTabsList will be updated to contain KListWithOverflow within itself at some point because that will resolve lots of issues across the whole Kolibri where we see tabs taking two or more rows. If it would help with this task, we could prioritize it and rather focus on updating KTabsList to support this behavior by default. I don't think it'd be complicated work, it's something we need to do anyways.

MisRob avatar Apr 17 '24 05:04 MisRob

This issue is blocked by this KDS issue - some of my feedback above re: tab styles/functionality will be added directly into KDS, so once that issue is closed, we can update this PR accordingly.

nucleogenesis avatar Apr 19 '24 15:04 nucleogenesis