Skip to content

fix(library): let long shelf names breathe in the Move To menu - #240

Merged
MaryWylde merged 1 commit into
devfrom
fix/library-move-to-dropdown-long-names
Oct 15, 2026
Merged

MaryWylde merged 1 commit into
devfrom
fix/library-move-to-dropdown-long-names

Conversation

@MaryWylde

Copy link
Copy Markdown
Contributor

Description

Long shelf names broke the Move To dropdown in the object overview modal. Two things were compounding:

  1. The portaled menu was pinned to the trigger's width. It got width: menuPos.width inline, and the Move To trigger is a compact width: fit-content pill (~90px) — so every shelf name was crushed into a one-or-two-words-per-line column.
  2. Long tokens escaped the box. Crime/Detective has no break opportunity in Chrome, and .menu is overflow: visible, so it painted straight over the modal content behind it.

What changed:

  • .menuPortal is now width: max-content, with inline minWidth (never narrower than the trigger) and maxWidth: min(320px, calc(100vw - left - 8px)) (never past the viewport's right edge, never stretched across the screen).
  • New .optionLabel on option and sub-option labels — overflow-wrap: anywhere, so an unbreakable name wraps inside the menu instead of over the page.
  • The trigger's own label truncates with an ellipsis (it's a fixed-height row; wrapping would spill vertically), and .moveToTrigger gets max-width: 100% — otherwise, once a shelf is picked the pill's label becomes that shelf's name and fit-content pushes it past the modal edge.

The library switcher (variant="library") is untouched: it isn't portaled, and it already truncates names via .libraryIdentity > p.

Verified in headless Chromium against the compiled CSS — normal names sit on one line inside the box, and a 70-character space-free name wraps rather than overflowing.

🔗 Related Issue

n/a — reported directly with a screenshot of the Move To menu.

✅ Checklist

  • My code follows the project's coding style
  • I have linked the relevant issue in the PR description
  • I have manually tested the changes — CSS verified in a browser harness; the live modal needs an authenticated owner session, so it wants one click-through before merge
  • All CI checks have passed

🤖 Generated with Claude Code

The portaled menu was pinned to its trigger's width, and the Move To
trigger is a compact fit-content pill — so shelf names were crushed into
a ~90px column, and a name with no break opportunity ("Crime/Detective")
walked straight out of the menu and over the modal behind it.

The portaled menu now sizes to its content: min-width keeps it at least
as wide as the trigger, max-width keeps it inside the viewport and off
the far edge of the screen. Option labels break long tokens instead of
escaping the box, and the trigger truncates its own label so a chosen
shelf name can't push the pill past the modal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Claude finished @MaryWylde's task in 2m 52s —— View job


Claude's review of #240

Todo list

  • Gather context (PR description, changed files)
  • Read diff (git diff origin/dev...HEAD)
  • Review Dropdown.tsx changes
  • Review Dropdown.module.scss changes
  • Review ObjectOverviewModal.module.scss changes
  • Post inline comments for issues found
  • Post final summary

Summary

This is a well-scoped, well-reasoned CSS/layout fix. The two root causes described in the PR body check out against the code:

  • .menu is width: 100%, which under the portaled position: fixed resolves against the viewport, not the trigger — switching .menuPortal to width: max-content with inline min-width/max-width is the right fix, and Dropdown.tsx:118-119 correctly derives the max-width from the live menuPos.left rather than a hardcoded value.
  • .optionLabel { overflow-wrap: anywhere } correctly targets the case where .menu is overflow: visible and a single unbreakable token would otherwise paint over the modal.
  • The moveToTrigger max-width: 100% addition is correctly scoped against its containing block (.dropdown, which is width: 100% of .row), and doesn't conflict with the existing width: fit-content !important.
  • Scope is appropriately narrow: the variant="library" switcher is untouched, matching the stated reasoning (it isn't portaled and already truncates via .libraryIdentity > p).
  • No violations of AGENTS.md conventions spotted: SCSS Modules only, classnames used correctly, Text component's className prop is passed through as expected, path aliases/import order unaffected, no App Router/Tailwind/state-library issues, no hardcoded colors.

Two low-severity, non-blocking edge cases left as inline comments for awareness (not requested as blocking changes):

  1. Dropdown.module.scss:158 — .subMenu sets white-space: nowrap (pre-existing, line 198), which is inherited by the new .optionLabel and suppresses overflow-wrap: anywhere entirely for sub-menu options. Currently dormant (the only sub-option consumer, Shelf.tsx, uses short static labels), but it means the wrap fix doesn't actually apply where the PR's own code wires it up (Dropdown.tsx sub-option Text).
  2. Dropdown.tsx:119 — the calc(100vw - left - 8px) can clamp to 0 if the trigger sits within 8px of the viewport's right edge, and separately, min-width (line 118) wins over max-width per spec if the trigger itself is ever wider than the available room — both unlikely with today's compact "Move To" pill, flagged only in case portal gets reused with a wider trigger.

Testing note: the PR body already flags that this was verified against compiled CSS in headless Chromium rather than the live authenticated modal — that gap is called out by the author, not something I could close either (no browser tooling available in this session).

// behind the dropdown. Break it instead.
.optionLabel {
min-width: 0;
overflow-wrap: anywhere;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overflow-wrap: anywhere is applied to .optionLabel, which is also used inside .subMenu for sub-option labels (Dropdown.tsx ~L210-215). But .subMenu sets white-space: nowrap (line 198), which is inherited by .optionLabel and takes precedence over overflow-wrap — white-space: nowrap suppresses all soft-wrap opportunities, including the emergency breaks overflow-wrap: anywhere would otherwise insert. So a long, unbreakable sub-option label would still overflow the sub-menu rather than wrap.

Not triggered by today's only sub-option consumer (Shelf.tsx's Private/Public privacy options are short), so this won't reproduce right now — but it means the fix is silently inert for sub-menus, which contradicts the stated intent ("Shelf and library names can be a single long token ... Break it instead") since sub-option labels can be arbitrary CMS-driven text too. Worth an explicit white-space: normal; (or removing nowrap from .subMenu and relying on min-width: 100% + overflow-wrap) so this doesn't resurface as a real bug once a long sub-option label ships.

Fix this →

// left of the viewport's right edge, and capped so a long shelf
// name can't stretch the menu across the whole screen.
minWidth: menuPos.width,
maxWidth: `min(320px, calc(100vw - ${menuPos.left + 8}px))`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two edge cases in this calc(), both low-severity but worth noting:

  1. If the trigger sits close enough to the right edge that 100vw - (menuPos.left + 8) goes negative (e.g. a resize/scroll that leaves less than 8px of room), CSS clamps a negative max-width to 0, which would make the portaled menu effectively invisible rather than just tight against the edge.
  2. minWidth: menuPos.width (line 118) and this maxWidth are independent — per spec, min-width wins over max-width when they conflict. So if the trigger itself is wider than the room available (e.g. a very long selected shelf name has grown the compact pill before the ellipsis truncation kicks in, or the trigger just happens to sit near a narrow viewport edge), the menu can still be forced wider than the viewport allows, reproducing a version of the original overflow bug.

Neither is likely with the current compact "Move To" pill in a centered modal, so this may not be worth guarding for now — flagging in case the portal variant gets reused somewhere with a wider trigger or edge-anchored placement.

@MaryWylde
MaryWylde merged commit 44ec72d into dev Oct 15, 2026
2 checks passed
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.

1 participant