Refactor KBreadcrumbs to utilize KListWithOverflow

[MisRob]

❌ This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.

In times when KListWithOverflow component didn't yet exist, we used the "duplicate markup" technique in the KBreadcrumbs component to achieve their overflow behavior when breadcrumb items were collapsed to the dropdown menu on the left:

The duplicate markup technique is based on keeping a hidden copy of visible breadcrumbs (breadcrumbs-offscreen) in the markup so that elements are available for recalculations:

https://github.com/learningequality/kolibri-design-system/blob/9d2a147de7e81c945ca0eab7ffb2c4719fd2bb06/lib/KBreadcrumbs.vue#L84-L118

Now that we have KListWithOverflow that is designed to take care of overflowing items to a menu, it would be good to remove this other implementation from KBreadcrumbs and refactor them to utilize KListWithOverflow instead.

The Value Add

Easier maintenance, development efficiency.

Guidance

Note that KListWithOverflow will likely need some updates to be able to display the button on the left side, and possibly some other adjustments so that these two components can collaborate smoothly.

Acceptance criteria

• [ ] There are no regressions in places where KBreadcrumbs and KListWithOverflow are used:
  • Live examples on their documentation pages
  • In Kolibri and Studio

[sruthin21]

Hey @MisRob I want to work on this Issue Please Assign this to me

[AlexVelezLl]

Hey @sruthin21! Thank you! I just assigned you this issue. Please let us know if you have any question. I was thinking to propose something for this issue but probably will be able to do that tomorrow :). But you can start looking at the issue for now.

[sruthin21]

@AlexVelezLl Thank You for assigning

[AlexVelezLl]

Hey @sruthin21! I am back with some things we will need for this:

1. We need to update KListWithOverflow since now we need to allow hiding overflowed items from the beginning, instead of from the end of the list as we courrently do. For this:

• We need to define a new prop overflowDirection that accepts as values start or end, with end as default value as this would be the current behaviour.
• We need to position the more button at the beginning or at the end of the list, depending on the overflowDirection prop.
• We need to modify the hiding elements logic inside the setOverflowItems method to support hiding elements from the beginning. This is probably the biggest challenge of the issue. I suggest to try to look for mathematical operations to achieve the reverse overflow instead of duplicating the logic for both overflowDirection values.

2. Update KBreadcrums to use KListWithOverflow.

• To prepare the items for this, we will need to add a { type: "separator" } object between every element, and use the #divider slot to render the "> " icon.
• We have this fixDividerVisibility method in KListWithOverflow to avoid having a separator at the beginning or at the end, and this will mess with the requirement of KBreadcrums to have the ">" icon just after the more button, so the easiest way to achieve that will be that the #more slot renders both, the more button, and the ">" separator icon. With this, we will avoid having to care about that.

There will problably be a lot of things that I am missing, so please let us know if you have any questions :hugs:.

[sruthin21]

What this throttle function do And what will be the output of the function

[AlexVelezLl]

This is just a way to optimize the number of calls we do to the setOverflowItems method, you can ignore it, and assume we will be always calling setOverflowItem each time the parent element is resized. You can read more about thottling here https://dev.to/jeetvora331/throttling-in-javascript-easiest-explanation-1081.

[sruthin21]

@AlexVelezLl I was able to fix the first task like adding the overflowDirection prop to the KListWithOverflow Componenet But unable to resolve the second task That is Updating KBreadcrums to use KListWithOverflow Component
Can you give the Idea to solve the second task And please tell me what the final output of the KBreadcrums component should look like

[AlexVelezLl]

Hey @sruthin21! Could you elaborate further on what problems you have experienced with the second point? The component should look more or less like this:

<!-- KBreadcrumbs.vue -->
<KListWithOverflow
  overflowDirection="start"
  :items="crumbs"
>
  <template #item="{ item }">
    <li>
      <KRouterLink
        v-if="item.link"
        :text="item.text"
        :to="item.link"
      >
        <template #text="{ text }">
          <span class="breadcrumbs-crumb-text">{{ text }}</span>
        </template>
      </KRouterLink>
      <span v-else>{{ item.text }}</span>
    </li>
  </template>
  <template #more="{ overflowItems }">
    <KIconButton
      size="small"
      icon="chevronUp"
      appearance="raised-button"
    >
      <template #menu>
        <KDropdownMenu
          :options="overflowItems"
        />
      </template>
    </KIconButton>
    <span>
      ›
    </span>
  </template>
</KListWithOverflow>

So the #item template should match the current visible breadcrumbs items, and we can use our dropodownmenu in the #more template. Having something like this in the template of KBreadcrumbs is the goal.

And please tell me what the final output of the KBreadcrums component should look like

For this you can see the Acceptance creteria in the issue description:

There are no regressions in places where KBreadcrumbs and KListWithOverflow are used:
* Live examples on their documentation pages
* In Kolibri and Studio

So basically as this is a refactor, the output should look exactly the same as it is right now, and we should make sure that we dont introduce bugs on something there was already working ok. We can check regressions in studio, and you can do it in Kolibri and in the docs pages. to test your changes in Kolibri you can check the how to preview updates in a product guide.

[sruthin21]

@AlexVelezLl I have made a pull request review it I know there are lots of changes to do

[MisRob]

Note the conversation above and the first attempt https://github.com/learningequality/kolibri-design-system/pull/790. Quite detailed review by @AlexVelezLl will serve as good guidance

[shruti862]

Hey @MisRob I want to work on this issue. Please Assign this to me

[MisRob]

Hi @shruti862, sure! Thank you. Note this is a bit more complex, but we already had one attempt and it was reviewed, so please check out the comments above as guidance.

[shruti862]

@MisRob I am done with the task-1 of the issue but i am facing issue in refactoring KBreadcrumb with updated KListWithOverflow ; I am unable to render '>' separator between items and to insert link in the text of dropdown menu in KBreadcrumb . Could you please review my code so that i get to know what i am doing wrong.I am raising a PR soon.

[MisRob]

Hi @shruti862, thanks! I converted your pull request to draft until it's finalized.

i am facing issue in refactoring KBreadcrumb with updated KListWithOverflow ; I am unable to render '>' separator between items and to insert link in the text of dropdown menu in KBreadcrumb

In your pull request, would you comment on some particular places that are related to this and be as specific as possible? We will need to hear more to understand what you're experiencing.

[shruti862]

@MisRob yes sure . I am facing issue in lib/KBreadcrumb.vue file ,I will make comments on this file regarding the issues i am facing. But can you please review KListWithOverflow to see if it is meeting all the requirements. In that file I tried to include all suggested changes AlexVelezLl suggested in referenced PR.

[MisRob]

Great @shruti862. I will invite @AlexVelezLl to assist there. The more information you can provide in the draft PR, the better. Thanks a lot for this work!

[MisRob]

Hi @shruti862, I wanted to mention that Learning Equality will be closed from December 23 to January 5.

[Abhishek-Punhani]

@MisRob Can i work on this issue ?

[MisRob]

Hi @Abhishek-Punhani, thank you! This is already assigned and @shruti862 actively works on it. You're welcome to find another issue. There are not many free ones now but we regularly add new 'help wanted' issues, so best to check on it from time to time.

[Abhishek-Punhani]

@MisRob Thank you for the clarification! I noticed that @shruti862 is already assigned to a few issues, so I just wanted to confirm if this one is currently being worked on. I’ll be happy to look for another issue if needed!

[AlexVelezLl]

Yes, they are @Abhishek-Punhani. @shruti862 actively working on this, and already opened a PR for this issue, what is pending is another code review from my side.

[MisRob]

@shruti862 I will unassign you as per https://github.com/learningequality/kolibri-design-system/pull/857#issuecomment-3056422427

[MisRob]

github.com/learningequality/kolibri-design-system/pull/857 has some good progress, but @AlexVelezLl noted a possible blocker in KListWithOverflow. This work needs some investigation and decisions on how to best proceed.