Refactor KBreadcrumbs to utilize KListWithOverflow

[sruthin21]

Description

Task1

1. Added OverFlowDirection Prop for the kListwithoverflow component
   Task2
2. Updated KBreadcrums to use KListWithOverflow.

Issue addressed

Addresses #693.

Before/after screenshots

Changelog

• Description: Summary of change(s)
• Products impact: Choose from - none (for internal updates) / bugfix / new API / updated API / removed API. If it's 'none', use "-" for all items below to indicate they are not relevant.
• Addresses: Link(s) to GH issue(s) addressed. Include KDS links as well as links to related issues in a consumer product repository too.
• Components: Affected public KDS component. Do not include internal sub-components or documentation components.
• Breaking: Will this change break something in a consumer? Choose from: yes / no
• Impacts a11y: Does this change improve a11y or adds new features that can be used to improve it? Choose from: yes / no
• Guidance: Why and how to introduce this update to a consumer? Required for breaking changes, appreciated for changes with a11y impact, and welcomed for non-breaking changes when relevant.

Steps to test

1. Step 1
2. Step 2
3. ...

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

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

Comments

[AlexVelezLl]

Hey @sruthin21! Thanks for your hard work. This is looking great! However my major concern is that we should make this change a non-breaking change, and avoid modifying the public API of the KBreadcrumbs and keep the external functionality the same as before.

One method to ensure that we are not breaking anything is to constantly check the KBreadcrumbs page http://localhost:4000/kbreadcrumbs and see that it looks and behave exactly the same as our current version of kbreadcrumbs which you can see in the production docs site https://design-system.learningequality.org/kbreadcrumbs/.

As you can see now we dont show the breadcrumbs in the docs page:

Before	After

[AlexVelezLl]

Here we should not change the public API as this would be a breaking change, we should keep the same API as before, or at least that if we make any change to the API, that this is not a breaking change. So we should keep the items and showSingleItem props, and they should keep working exactly the same as before the refactor.

[AlexVelezLl]

Here we dont need to import there components, as they are already registered in the Vue context, we can use it without it.

[AlexVelezLl]

We are not doing anything with this visibleItems array. I would suggest to remove them. Also I dont note any added value of removing the maxWidth = Math.ceil(maxWidth); sentence. I think we can keep these lines as before:

        maxWidth = Math.ceil(maxWidth);
        this.overflowItems = overflowItemsIdx.map(idx => this.items[idx]);
        this.isMoreButtonVisible = overflowItemsIdx.length > 0;
        list.style.maxWidth = `${maxWidth}px`;
        list.style.maxHeight = `${maxHeight}px`;
      },

[AlexVelezLl]

We dont need to reverse the itemSizes because we are already reverting the directionIndexes, and when we iterate directionIndexes, we are using the same index to access the list.children which is not reverted and the itemSizes which is reverted. So we are not matching the same information for the children and its size.

[AlexVelezLl]

I think its better to revert this change and have the item object to avoid repeating the list.children[i]

[AlexVelezLl]

Lets keep these comments, as they are useful for new team members that doesnt know the logic here

[AlexVelezLl]

We need to also check the logic of the fix dividers Visibility, because for example here, when we have dividers, we can have this situation where some overflowed items are visible:

Compartir.pantalla.-.2024-09-28.10_26_02.mp4

[AlexVelezLl]

We cant have this solution for the position of the button, because Keyboard navigation would not be the same. We need to keep the current behaviour in KBreadcrumbs that if the more button appears at the beginning, the keyboard navigation should focus it first, before the rest items.

Compartir.pantalla.-.2024-09-28.10_29_00.mp4

[AlexVelezLl]

Here we shouldnt add color styles like these. We cant set burned colors, instead we need to use our brand colors, you can find the guides here.

Although, I think most of these styles arent really needed. Do they?

[AlexVelezLl]

Also, a side note! When we open a PR we should keep and fill the form template that appears when we create the PR, not remove it, I have pasted it again to the PR description 🤗.

[AlexVelezLl]

Hey @sruthin21! Are you still interested in continuing working in this issue?

[MisRob]

Hi @sruthin21, by deleting your fork, unfortunately we won't be able to finish this PR. I will unassign you from #693, but feel free to message us and re-open if you'd like.