Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActionList.Group should allow for TrailingActions #2043

Open
mattcosta7 opened this issue Apr 22, 2022 · 15 comments
Open

ActionList.Group should allow for TrailingActions #2043

mattcosta7 opened this issue Apr 22, 2022 · 15 comments

Comments

@mattcosta7
Copy link
Collaborator

mattcosta7 commented Apr 22, 2022

Describe the bug

ActionList.Group should allow for a TrailingAction, potentially with an api similar to this one, to maintain consistency with ActionList.Item

<ActionList.Group>
  <ActionList.GroupHeader>
    Title
    <ActionList.TrailingVisual>
      {/* reviewer dropdown */}
    </ActionList.TrailingVisual>
  </ActionList.GroupHeader>

  <ActionList.Item>
    {/* ... */}
  </ActionList.Item>

  {/* ... */}
</ActionList.Group>

Expected behavior

A user can define a trailing action for a group which appears as a button/link on the end of the group header, similar to the provided sample screenshot

Screenshots

ActionGroupSample

Additional context

Slack thread

@lesliecdubs
Copy link
Member

Thanks for filing! We're going to explore the accessibility implications of this and will get back to you.

@siddharthkp
Copy link
Member

siddharthkp commented Apr 26, 2022

Leaving notes for the future:

The mock up here is a perfect use case for NavigationList, so we could pick this up with the upcoming work. cc @colebemis

Because NavigationList uses ActionList under the hood, we would need to add this to ActionList.

However, we should add safe guards in place for ActionMenu (which also uses ActionList) because a button next to group title inside role=group would need different semantics (maybe even a different design pattern).

For now, we can choose to simply no allow a trailingAction inside an ActionMenu Group

@mattcosta7
Copy link
Collaborator Author

Definitely @siddharthkp ! We actually mocked this up as an implementation of navigationlist since there wasn't a component for it yet :-)

@github-actions
Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Oct 23, 2022
@mattcosta7
Copy link
Collaborator Author

Not stale

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2023

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Oct 8, 2023
@mattcosta7 mattcosta7 removed the Stale label Oct 9, 2023
@dylanatsmith
Copy link
Contributor

Adding some additional use cases:

Search workflows

This is live in the repo Actions tab (ex: https://github.com/github/github/actions)

image

Manage knowledge bases

This is from a new design for Copilot chat

image

Copy link
Contributor

Uh oh! @dylanatsmith, the image you shared is missing helpful alt text. Check #2043 (comment).

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.

Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Aug 12, 2024
@dylanatsmith
Copy link
Contributor

Ha, this actually came up today due to the ✏️ on the categories sidebar in Discussions.

Image

Copy link
Contributor

Uh oh! @dylanatsmith, the image you shared is missing helpful alt text. Check #2043 (comment).

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 github-actions bot removed the Stale label Aug 12, 2024
@lesliecdubs
Copy link
Member

Thanks for letting us know @dylanatsmith, and sorry this is coming up for you again!

We're going to bring this up at the next Primer Patterns session (thank you @mperrotti!) to discuss whether this change is something we'd want to bake in. Feel free to join if you're able/want to; you can request an invite in #primer on Slack.

@mperrotti
Copy link
Contributor

@dylanatsmith @siddharthkp - during the Primer patterns session we agreed this is a good pattern, but we're not sure if it's worth building into the component.

What's the benefit of handling this in Primer instead of having consumers do it manually? Here's an example of how it would be done manually:

<ActionList.GroupHeading as="h3">
  <Stack direction="horizontal" align="center">
    <Stack.Item grow>Repositories</Stack.Item>
    <Stack.Item>
      <ActionList.TrailingAction label="Expand sidebar" icon={ArrowLeftIcon} />
    </Stack.Item>
  </Stack>
</ActionList.GroupHeading>

@dylanatsmith
Copy link
Contributor

What's the benefit of handling this in Primer instead of having consumers do it manually?

Consistency I guess. Is it a button with a label, an IconButton, a button with an icon and a label? Is it small or default size? Does the engineer align it properly? What about some links? If they smash a whole Dialog or other elements inside of that h3 are we in a bad place for accessibility? etc.

But I don’t feel strongly, I’m good with whatever decision you all make. Some of those questions above probably point toward not being able to support the range of use cases with the component alone anyway.

@En09

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants