Skip to content

fix(widget): prevent thread drawer children from shrinking - #145

Open
Karn2898 wants to merge 2 commits into
extra-org:mainfrom
Karn2898:fix/widget-thread-drawer-flex-shrink
Open

Karn2898 wants to merge 2 commits into
extra-org:mainfrom
Karn2898:fix/widget-thread-drawer-flex-shrink

Conversation

@Karn2898

Copy link
Copy Markdown
Contributor

What is wrong

In the widget's chat history drawer ("Chats"), conversation rows get squashed instead of scrolling once there are many conversations. Each row shrinks below its natural height until only the padding is left, so titles get clipped at the top and bottom and rows visually overlap. With ~20 conversations there is no scrollbar at all.

Why it happens

.thread-list is a display: flex; flex-direction: column container. Its children .thread-drawer-head, .thread-new, and .thread-item never set flex-shrink, so they keep the CSS default flex-shrink: 1 and the browser shrinks them to make the content fit the container — instead of overflowing and triggering overflow-y: auto.

Suggested fix

Opt the drawer's children out of shrinking:

.thread-drawer-head, .thread-new, .thread-item { flex: 0 0 auto; }

Things to watch out for

  • .thread-item uses white-space: nowrap; overflow: hidden; text-overflow: ellipsis, which hides the symptom horizontally but not the vertical clipping — the row boxes themselves are too short.
  • .thread-list already has the correct flex: 1; min-height: 0; overflow-y: auto; the bug is one level down, in the children.
  • .messages (the message list) has the same flex: 1; min-height: 0; overflow-y: auto pattern, but its content wrapper .conversation-content uses min-height: 100%, so it is not affected.
  • src/agent_manager/api/static/widget.js is the built bundle and contains a copy of this CSS — rebuilt after the fix.
  • A regression test was added: render the drawer with 25 conversations and assert the row height does not depend on the count and that the list overflows.

Verification

  • npm run build:widget — bundle rebuilt with the three rules.
  • npm run typecheck:widget — clean.
  • npm run test:widget — widget self-check: OK.
  • npx playwright test — 41 passed, including the new regression test.
  • The new test fails when the fix is reverted (rows squashed to ~25px) and passes with the fix restored.

The chat-history drawer's flex children (.thread-drawer-head, .thread-new,
and .thread-item) left flex-shrink at its CSS default of 1, so the browser
compressed them to fit the panel once the list overflowed. Rows shrank below
their natural height, clipping titles and hiding the scrollbar.

Opt each drawer child out of shrinking with flex: 0 0 auto so the
scrollable .thread-list keeps its rows at natural height and scrolls.

Add a Playwright regression test asserting row height is independent of
the number of conversations and that the list overflows.

Closes the thread-drawer squashing bug.
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the implementation itself looks good and the fix is scoped correctly.

Using flex: 0 0 auto on the drawer header, new-chat button, and thread items is the right fix here. The regression test also covers the important behavior by checking that row heights stay stable and that the thread list actually overflows.

There is just one small CI issue left.

The check job is green, but the ui job fails during typecheck:e2e with:

TS18047: 'scrollMetrics' is possibly 'null'

This comes from:

expect(scrollMetrics).not.toBeNull();
expect(scrollMetrics.scrollHeight).toBeGreaterThan(scrollMetrics.clientHeight);

TypeScript does not narrow the type based on the Playwright assertion.

I would make the evaluated code fail explicitly if .thread-list is missing, so scrollMetrics is always non-null:

const scrollMetrics = await (await widget(page)).evaluate((el) => {
  const list = el.shadowRoot?.querySelector(".thread-list");
  if (!list) throw new Error("thread-list not found");

  return {
    scrollHeight: list.scrollHeight,
    clientHeight: list.clientHeight,
  };
});

expect(scrollMetrics.scrollHeight).toBeGreaterThan(scrollMetrics.clientHeight);

After that, please rerun CI.

No production logic changes are needed from my side — once CI is green, this looks ready to approve.

This branch has not been deployed

No deployments
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