learningequality / kolibri-design-system

Kolibri Design System
https://design-system.learningequality.org
30 stars 73 forks source link

Refactor KBreadcrumbs to utilize KListWithOverflow #693

Open MisRob opened 3 months ago

MisRob commented 3 months ago

🌱 Are you new to the codebase? Welcome! Please see the contributing guidelines.

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:

breadcrumbs

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

sruthin21 commented 2 months ago

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

AlexVelezLl commented 2 months ago

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 commented 2 months ago

@AlexVelezLl Thank You for assigning

AlexVelezLl commented 2 months ago

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 commented 2 months ago

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

AlexVelezLl commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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