react icon indicating copy to clipboard operation
react copied to clipboard

Add Storybook documentation on support for keyboard bindings and multiple trailing icons on Buttons

Open arisacoba opened this issue 6 months ago • 8 comments

Buttons should support keyboard bindings and allow adding multiple trailing icons, such as those used to indicate keyboard shortcuts (e.g., ⌘K or ⌘⏎). This would help users easily identify and use keyboard shortcuts and would make the components more flexible for different use cases.

Sample from issue creation inside Copilot immersive

Image

arisacoba avatar Jul 07 '25 11:07 arisacoba

Uh oh! @arisacoba, at least one image you shared is missing helpful alt text. Check your issue body to fix the following violations:

  • Images should have meaningful alternative text (alt text) at line 5

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

github-actions[bot] avatar Jul 07 '25 11:07 github-actions[bot]

@dipree mentioned that this is possible already: https://ui.githubapp.com/storybook/?path=/story/primer_experimental-components-keybindinghint-examples--primary-button

However we don't have an example for this in the primer storybook, so we should add this in a story.

lukasoppermann avatar Jul 07 '25 14:07 lukasoppermann

as @lukasoppermann mentions, rewording this issue to capture the documentation gap

hellojanehere avatar Jul 07 '25 17:07 hellojanehere

We should also support this for disabled buttons. This is how it looks right now if the button is disabled

Image

arisacoba avatar Jul 08 '25 07:07 arisacoba

Uh oh! @arisacoba, at least one image you shared is missing helpful alt text. Check https://github.com/primer/react/issues/6290#issuecomment-3047767383 to fix the following violations:

  • Images should have meaningful alternative text (alt text) at line 3

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

github-actions[bot] avatar Jul 08 '25 07:07 github-actions[bot]

@arisacoba this is definitely a bug, however, I think this is a bug in the keyBindingHints.

@hellojanehere I feel we should consider upstreaming the keyBindingHints to primer to have control over cases like this.

lukasoppermann avatar Jul 08 '25 08:07 lukasoppermann

interesting @lukasoppermann - who owns this prop currently?

hellojanehere avatar Jul 08 '25 08:07 hellojanehere

@hellojanehere it seems like we actually do. It is coming from primer/experimental.

So we do have control and can easily fix this. However, I still feel like we should upstream this from primer recipes to primer.

lukasoppermann avatar Jul 08 '25 08:07 lukasoppermann