Klistwithoverflow component

[AlexVelezLl]

Description

Introduces a new component Klistwithoverflow that renders an horizontal list that automatically renders a "show more" component if any items overflow the container.

Issue addressed

Addresses #556

Before/after screenshots

Compartir.pantalla.-.2024-02-27.09_35_51.mp4

Changelog

• #552
  • Description: New KListWithOverflow component.
  • Products impact: new API.
  • Addresses: List Component with Dynamic Overflow Handling #556, Implement "Command Palette" for editing multiple resources studio#3423, Setup wizard: Responsive language list kolibri#11923.
  • Components: KListWithOverflow.
  • Breaking: no.
  • Impacts a11y: no.
  • Guidance: -.

• #552
  • Description: New useKResponsiveElement private composable, KResponsiveElementMixin translated to this composable.
  • Products impact: -.
  • Addresses: -.
  • Components: -.
  • Breaking: no.
  • Impacts a11y: no.
  • Guidance: -.

Steps to test

1. Run the development server.
2. Go to http://localhost:4000/klistwithoverflow.
3. Resize the window or change examples containers widths, and check that the list is responsive.

Implementation notes

At a high level, how did you implement this?

1. Leave the list of items as flex-wrap: nowrap and overflow: visible so that all the items are on a single line without taking more height and without showing the overflow bar.
2. To control which items are seen and which are not, we set its visibility to hidden or visible, this allows us to know the width of all the items without the user seeing them.
3. Based on the widths of the items, I fill the available space, and set its visibility depending on whether they fit or not.
4. We accumulate the widths of the visible items and set a max-width to the list so that it takes only the required space and does not affect the positioning of the more button.

Testing checklist

• Contributor has fully tested the PR manually
• If there are any front-end changes, before/after screenshots are included
• 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

[marcellamaki]

I have only done an initial read through of this, @AlexVelezLl, and will do a more thorough review tomorrow with more comments and notes, but this looks like really great work! 🎉 I'm excited :)

[MisRob]

Hi @AlexVelezLl, overall the interface is looking great and flexible, wonderful work on research and implementation. This will be very helpful for many components across all products.

I will submit a full review later on.

One thing I'd like to suggest at the outset is that we don't use ResizeSensor in useKResponsiveElement. KResponsiveElement's usage is discouraged due to performance issues https://design-system.learningequality.org/kresponsiveelement/#guidelines. From what I know, I think that copying the same logic to useKResponsiveElement (and subsequently to each instance of KListWithOverflow which will eventually be used very frequently, e.g. in many cards on one page) would likely cause performance bottlenecks.

Since we won't need to support IE anymore, I think we could try to use ResizeObserver? Or you're welcome to research other alternatives. If you were going to use the ResizeObserver, please note that using a single ResizeObserver instance is recommended to observe multiple elements (source).

[AlexVelezLl]

Thank you @MisRob! Yes, I've worked before with ResizeObserver but it has 96.7% browser support, so I saw the way KBreadcrumbs accomplished its responsiveness and realized about the ResizeSensor and thought it was the standard way to do it.

So, I'll use the ResizeObserver but will add a check to add backwards compatibility for older browsers and avoid it if its undefined. In the end in the first render we would still have the responsiveness, we just lose the reactivity when some element is resized (which are fewer use cases).

A solution if issues arise due to this behavior (not recalculating the overflowItems when item resizing in older browser) could be to hide the entire list and only render the "more button", which would allow it to remain functional.

[MisRob]

@AlexVelezLl We don't need to support all browsers. There are actually plans to remove IE11 specific logic from KDS, so it'd be best not to be adding support for it or other browsers that are not needed. Please see the Kolibri browserlist and if all these browsers support ResizeObserver, let's not use ResizeSensor at all (and rather plan to remove it completely from the codebase at some point)

[MisRob]

I saw the way KBreadcrumbs accomplished its responsiveness and realized about the ResizeSensor and thought it was the standard way to do it.

It used to be, yes. The removal of IE11 support is a pretty recent update that will take effect in Kolibri 0.17. Generally, Kolibri has the most strict requirements in our ecosystem so its browserlist is the thing to watch for.

[MisRob]

One thing to watch for in this period of transition is not to use this logic in Kolibri 0.16, for example not to refactor any components to use it. I don't think we will do that :) But will make sure to mention it in the release notes.

[AlexVelezLl]

Thanks @MisRob, I just updated the PR with the ResizeObserver.

Please see the Kolibri browserlist and if all these browsers support ResizeObserver, let's not use ResizeSensor at all (and rather plan to remove it completely from the codebase at some point).

Yes, it seems like not all Kolibri's supported browsers support ResizeObserver. I just added this check to avoid any null exception. This way we just dont provide the reactivity when the element is resized if the browser dont have support for ResizeObserver.

[AlexVelezLl]

One thing to watch for in this period of transition is not to use this logic in Kolibri 0.16

I was planning to use it for the languageSwitcher in the Kolibri's setup wizard learningequality/kolibri#11923. Do you mean not including this in future 0.16 patches?

[MisRob]

I was planning to use it for the languageSwitcher in the Kolibri's setup wizard learningequality/kolibri#11923. Do you mean not including this in future 0.16 patches?

I don't see a milestone yet for learningequality/kolibri#11923 yet. So to sum it up:

(1) If the issue is planned for 0.16 => we need IE11 support in this component now and will remove it later
(2) If the issue is planned for 0.17 => no need for IE11 support

(2) will is simpler, but if it's important to have it in 0.16, we can add IE fallback now as you suggested and remove it later.

I'd leave you decide together with @marcellamaki about the milestone for the issue.

[AlexVelezLl]

Got it! I dont think its super urgent tho

[MisRob]

@AlexVelezLl Overall, this is great work, very clear and flexible component design and I appreciate your comments. I am not going to pretend I fathomed all the calculations in detail but as a whole it makes sense to me and I didn't notice anything suspicious. Posting a few comments and a question or two.

[MisRob]

And one changelog update request, please let's communicate it's a private composable here and that products impact is none rather than new API since it's not public.

[MisRob]

I think that's all from my first review round

[AlexVelezLl]

Thank you @MisRob! Comments addressed :).

[MisRob]

Thanks @AlexVelezLl, good work here.