#680 | feat(masonry): improve Palette interactions - #774
Open
yasumorishima wants to merge 1 commit into
Open
Conversation
Covers the four objectives in sugarlabs#680. Pointer cursor on the clickable items. The Button primitive sets no cursor, and a `button` gets `cursor: default` from preflight, so the sidebar category buttons and the classification tabs showed an arrow. Both now carry `cursor-pointer`. Bricks are left on `cursor-grab`, which is what they are. Cursor limited to the Brick. The drag source in `BrickSlot` was a block-level flex row, so it spanned the full width of the list and the grab cursor appeared over the empty space beside a Brick. `w-fit` shrinks it to the Brick, which also narrows the drag target to the Brick itself. Title on the section buttons. The sidebar label is truncated when the sidebar is narrow, so the button now carries the category name as a `title`, matching what the classification tabs already do. Feedback when the list cannot scroll. Clicking a category button whose section is already at the end of the scroll container left `scrollIntoView` with nothing to do and the click read as ignored. The target header now highlights briefly (600 ms) on every click, so the click is always answered; the marker is exposed as `data-flashing` so it can be asserted. The pending timeout is cleared when a different classification is selected and on unmount. Adds five tests: the sidebar title attribute, the flash appearing, the flash clearing once elapsed, only the clicked category being marked, and a pending flash not surviving a classification switch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAbG8QN1gYq5axi6KUopBj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #680.
Implements all four objectives, plus tests.
1. Pointer cursor on the clickable items
buttonVariantsinui/button.tsxsets no cursor, and abuttonelement takescursor: defaultfrom preflight, so the sidebar category buttons and the classification tabs showed an arrow rather than a pointer. Both now carrycursor-pointer.Deliberately not changed: the bricks keep
cursor-grab/active:cursor-grabbing— they are drag sources, not click targets, so a pointer would misdescribe them. Also left alone:buttonVariantsitself. Fixing it there would be the smaller diff, but it would change every button in the app, which is wider than "the Palette component". Say the word if you would rather have it at the primitive.2. Cursor limited to the Brick
The drag source in
BrickSlotwas a block-level flex row, so it stretched the full width of the list and the grab cursor appeared over the empty space beside a Brick.w-fitshrinks it to the Brick. This also narrows the drag target to the Brick itself, which is the same intent read from the other direction — worth a look in case you want the wider row to stay draggable.3. Title on the section buttons
The sidebar label is
truncated inside aw-20column, so a longer category name is cut off with no way to read it. The button now carries the category name as atitle, matching what the classification tabs already do. It does not affect the accessible name, which still comes from the visible label.4. Feedback when the list cannot scroll
Clicking a category button whose section is already at the end of the scroll container left
scrollIntoViewwith nothing to do, so the click read as ignored — the case the issue calls out.The target header now highlights briefly (600 ms) on every click rather than only when the scroll is a no-op. Detecting "the scroll did nothing" means comparing scroll offsets across a smooth-scroll animation, which is fragile and, in jsdom, not testable at all; flashing unconditionally answers the click in both cases and is much simpler to reason about. If you would rather it fire only in the no-op case, I will take another pass.
The marker is exposed as
data-flashingso the behaviour can be asserted without reaching into class names. The pending timeout is cleared when a different classification is selected and on unmount.Tests
Five added, following the existing groupings in
Palette.test.tsx:title.Verification
Run on a fork of this repository at the same commit — all four jobs green:
Verify TypeScript types,Run Tests on Source Code,Run Smoke Test (build),Lint Code Base.Checked by hand as well:
px-1and cancels it with-mx-1, so the icon and heading do not shift.data-flashingisundefinedwhen inactive, so React omits the attribute rather than renderingdata-flashing="false".react-hooks/exhaustive-depshas nothing to flag.Not verified: how the flash actually looks in a browser. I could not exercise the UI visually, so the 600 ms duration and the
bg-mutedtint are a judgement call rather than something I watched — worth a glance from someone who can run it.