Skip to content

Fix KListWithOverflow restored items overlapping and wrap-flicker#1246

Open
nucleogenesis wants to merge 2 commits intolearningequality:developfrom
nucleogenesis:studio/5258-rte--dynamic-width/klistwithoverflow-wrap-fix
Open

Fix KListWithOverflow restored items overlapping and wrap-flicker#1246
nucleogenesis wants to merge 2 commits intolearningequality:developfrom
nucleogenesis:studio/5258-rte--dynamic-width/klistwithoverflow-wrap-fix

Conversation

@nucleogenesis
Copy link
Copy Markdown
Member

@nucleogenesis nucleogenesis commented Apr 28, 2026

🤖 AI usage: Claude Code (Opus 4.7) traced the two bugs in KListWithOverflow.setOverflowItems() and authored the fixes and this PR description at my direction. I reviewed the analysis, paired the change against studio#5707 via a local pnpm link, and manually verified the fixes in the toolbar at the failing widths and during fast resize. Any errors are mine.

Description

Two related bugfixes in KListWithOverflow, both surfaced while integrating it in the Studio RTE toolbar in learningequality/studio#5707.

Issue addressed

learningequality/studio#5258 — discovered while integrating KListWithOverflow for dynamic toolbar overflow.

Before/after screenshots

The clearest manifestation is at viewport widths around 900–940px in the Studio RTE toolbar, where the algorithm's "could everything fit if we hid the More button?" branch fires and restores items to visibility: visible while leaving them position: absolute. Restored items paint at the top-left of .list, on top of the first visible group.

Width Before (KDS develop) After (this PR)
900px before-900 after-900
940px before-940 after-940

In the "before" shots, look at the toolbar's text-formatting cluster: the B / I / U / S icons are stacked under format-direction / clear-format icons rather than sitting in their own slots. In "after", every icon is legible and distinct.

The wrap-flicker fix is harder to capture in a still image — it's only visible during active resize, between the browser reflowing and the throttled setOverflowItems() running. Easiest reproduction: drag-resize the viewport quickly across the overflow threshold; before the fix you'll see the toolbar briefly grow to two rows, after the fix it stays one row.

Changelog

  • Description: Reset position: 'unset' when restoring items in the "could everything fit if we hid the More button?" branch of setOverflowItems(). Without this, restored items remained position: absolute (set earlier by the overflow loop) and painted at the top-left of .list, overlapping the first visible item. The misbehavior is most visible at viewport widths where exactly the restoration branch fires (e.g. ~850–950px in the Studio toolbar case).

  • Products impact: bugfix

  • Addresses: learningequality/studio#5258, learningequality/studio#5707

  • Components: KListWithOverflow

  • Breaking: no

  • Impacts a11y: no

  • Guidance: -

  • Description: Prevent visible wrap-flicker on resize by setting flex-wrap: nowrap and overflow: hidden on the inner .list. Previously items would briefly wrap to a second row before the throttled overflow algorithm caught up, making the list jump in height.

  • Products impact: bugfix

  • Addresses: learningequality/studio#5258, learningequality/studio#5707

  • Components: KListWithOverflow

  • Breaking: no

  • Impacts a11y: no

  • Guidance: Consumers carrying their own ::v-deep .list { overflow: hidden } workaround for resize-flicker (such as studio#5707 currently does) can drop it after upgrading.

Steps to test

  1. Set up a KListWithOverflow consumer in a flexible-width container (the docs example, or pull Replace hardcoded toolbar breakpoints with KListWithOverflow studio#5707 with a local link to this branch).
  2. Resize the viewport through the threshold where exactly one item just barely overflows. Confirm the would-be-overflowed item does not visually overlap the first visible item.
  3. Drag-resize the viewport quickly across the overflow threshold. Confirm items do not briefly wrap to a second row before snapping back.
  4. Verify normal overflow still works at narrower widths (More button shows, dropdown lists overflowed items correctly).
  5. Verify the divider-visibility logic still applies (the divider before the first overflowed item is hidden).

(optional) Implementation notes

Bug 1 — restored items overlap visible items

setOverflowItems() runs in two passes. The overflow loop marks items as overflowed by setting visibility: hidden; position: absolute. The restoration pass then asks "would everything fit if we hid the More button?" and, if yes, re-shows items by setting visibility: visible. The bug: the restoration loop never resets position to 'unset' (the way the non-overflow branch does). Restored items remain absolutely positioned at (0, 0) of .list (which is position: relative) and paint over the first visible item.

Fix: one line — set item.style.position = 'unset' in the restoration loop.

This was introduced in #612, which added the position: absolute line in the overflow path but didn't update the restoration path to match.

Bug 2 — wrap-flicker on resize

The component throttles setOverflowItems() at 100ms via lodash. On resize the browser reflows immediately while the JS update is delayed; in that window items wrap to a new row because .list was flex-wrap: wrap; overflow: visible. The list briefly grows two rows tall, then snaps back when JS hides the overflowed items.

Fix: flex-wrap: nowrap; overflow: hidden on .list. With nowrap, items can't wrap visibly. overflow: hidden clips horizontal extension during the brief pre-JS moment (since children have flex-shrink: 0, the row would otherwise smear past the box).

The algorithm itself is untouched. Items are still measured via getBoundingClientRect() and removed from layout via position: absolute. maxWidth/maxHeight are still set on .list after each pass.

Does this introduce any tech-debt items?

No. The component still lacks unit tests (tracked in #833). Adding focused tests for these two cases would be ideal but is intentionally out of scope here.

Testing checklist

  • Contributor has tested the PR manually in the studio#5707 toolbar via local link
  • If there are any front-end changes, before/after screenshots are included
  • Critical and brittle code paths are covered by unit tests (out of scope, see [KListWithOverflow]: Add unit tests #833)
  • The change is described in the changelog section above

Reviewer guidance

  • Verify the algorithm change in setOverflowItems() (one position: 'unset' line) does not regress any other consumer (KBreadcrumbs, language switcher, etc.).
  • Verify the CSS change (nowrap + overflow: hidden on .list) doesn't visually change consumers that previously relied on wrap behavior.
  • LTR / RTL parity unchanged (no text-direction-sensitive code touched).

Comments

Drafted while the paired Studio PR (#5707) is also still in review. Once this lands and is released, the studio PR can drop its ::v-deep .list { overflow: hidden } workaround and bump the KDS version.

Two related fixes in `KListWithOverflow.vue`:

1. The "could everything fit if we hid the More button?" branch in
   `setOverflowItems()` restored items by setting `visibility: visible`
   but never reset `position: 'unset'` (the overflow loop above had set
   it to `'absolute'`). Restored items therefore painted at (0, 0) of
   `.list` (its position is `relative`), overlapping the first visible
   item — most visibly at viewport widths where the algorithm hits
   exactly that branch (around 850-950px in the Studio RTE toolbar
   case, learningequality/studio#5707).

2. `.list` previously used `flex-wrap: wrap; overflow: visible`. On
   resize the throttled `setOverflowItems()` runs after the browser has
   already reflowed; in that window items wrapped to a new row and
   briefly made the toolbar two rows tall before snapping back. Switch
   to `flex-wrap: nowrap; overflow: hidden` so items can't wrap visibly
   and any horizontal overflow is clipped during the brief moment
   before `maxWidth` is recomputed.

The algorithm itself is unchanged: items are still measured via
`getBoundingClientRect()` and removed from layout via `position:
absolute`. The CSS change just stops the temporary wrap from being
painted; the position-reset just completes the restoration path that
was missed in learningequality#612.
nucleogenesis added a commit to nucleogenesis/studio that referenced this pull request Apr 29, 2026
Adds a `pnpm.overrides` entry pointing `kolibri-design-system` at the
fork branch backing the paired KDS PR (learningequality/kolibri-design-system#1246).
This lets reviewers `pnpm install` and immediately exercise the toolbar
with both Studio's learningequality#5707 changes AND the upstream KDS fix, instead of
needing to set up a `pnpm link` against a local KDS checkout.

REMOVE BEFORE MERGE:
- Delete the `kolibri-design-system` entry under `pnpm.overrides` in
  `package.json`.
- Bump the `kolibri-design-system` dependency version above to whatever
  KDS release ships with learningequality#1246 included.
- Run `pnpm install` to regenerate `pnpm-lock.yaml`.

This commit should be the first thing reverted/dropped once the KDS PR
is released.
@nucleogenesis nucleogenesis marked this pull request as ready for review April 29, 2026 05:18
@AlexVelezLl AlexVelezLl self-assigned this Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants