Skip to content

feat: Add missing hold affordance icon for longPress S2 MenuTriggers - #10066

Merged
LFDanLu merged 6 commits into
mainfrom
s2_menu_long_press
May 15, 2026
Merged

feat: Add missing hold affordance icon for longPress S2 MenuTriggers#10066
LFDanLu merged 6 commits into
mainfrom
s2_menu_long_press

Conversation

@LFDanLu

@LFDanLu LFDanLu commented May 14, 2026

Copy link
Copy Markdown
Member

Closes #10029

Adds the hold affordance icon to "longPress" MenuTriggers in S2. Supports ActionButton and ToggleButton triggers

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Test the new story and see chromatic.
Also test the pre-existing docs and make sure the affordance icon and behavior works as expected.

🧢 Your Project:

RSP

@rspbot

rspbot commented May 14, 2026

Copy link
Copy Markdown

@LFDanLu

LFDanLu commented May 14, 2026

Copy link
Copy Markdown
Member Author

@snowystinger snowystinger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

docs?

*/
isQuiet?: boolean;
/** @private */
holdAffordance?: boolean;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just add to the context interface?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh good point, I guess I could use the same context in the Chromatic instead of using the prop directly too

}
};

export const HoldAffordance: Story = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe we make it look a bit more like real ones?

Here's how photoshop uses something similar
Image

@LFDanLu

LFDanLu commented May 14, 2026

Copy link
Copy Markdown
Member Author

@snowystinger docs actually already exist for this under https://d1pzu54gtk2aed.cloudfront.net/pr/20baef5c94eb4c797683a21575ec75d1b33e330c/ haha, definitely easy to miss. I didn't realize the same thing until I wrote some new docs and went through a whole debugging session ugh

I'll put the above in the test instructions too

@rspbot

rspbot commented May 14, 2026

Copy link
Copy Markdown

snowystinger
snowystinger previously approved these changes May 14, 2026
reidbarber
reidbarber previously approved these changes May 15, 2026
Comment on lines +64 to +65
import {ImageContext} from './Image'; // chevron right removed??
import InfoCircleIcon from '../s2wf-icons/S2_Icon_InfoCircle_20_N.svg';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

misplaced comment? Or should this be removed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ugh, must of been the formatter or auto import moving things around, thanks for the catch

@LFDanLu
LFDanLu dismissed stale reviews from reidbarber and snowystinger via 89fbda8 May 15, 2026 16:59
@rspbot

rspbot commented May 15, 2026

Copy link
Copy Markdown

@LFDanLu
LFDanLu added this pull request to the merge queue May 15, 2026
Merged via the queue into main with commit 69fc5b8 May 15, 2026
29 checks passed
@LFDanLu
LFDanLu deleted the s2_menu_long_press branch May 15, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spectrum 2 toggle button in react missing the hold icon/functionality

5 participants