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.TrailingAction does not fit ActionList.Item parent #5668

Open
prichodko opened this issue Feb 7, 2025 · 7 comments
Open

ActionList.TrailingAction does not fit ActionList.Item parent #5668

prichodko opened this issue Feb 7, 2025 · 7 comments
Assignees
Labels
bug Something isn't working react

Comments

@prichodko
Copy link

Description

The ActionList.TrailingAction component within ActionList.Item is causing a layout issue. Specifically, the underlying IconButton (32px) exceeds the available height of 20px, which causes unwanted expansion of the wrapping element..

Image

Here's the current component structure:

  <ActionList.Item key={thread.id} onSelect={() => manager.selectThread(thread)}>
    <ActionList.LeadingVisual>
      <CommentIcon />
    </ActionList.LeadingVisual>
    {threadName(thread)}
    <ActionList.Description variant="inline">
      <RelativeTime date={new Date(Date.parse(thread.updatedAt))} format="relative" />
    </ActionList.Description>
    <ActionList.TrailingAction
      icon={TrashIcon}
      label={`Delete conversation: "${threadName(thread)}"`}
      onClick={async () => {}}
    />
  </ActionList.Item>

Steps to reproduce

https://codesandbox.io/p/sandbox/magical-zhukovsky-3wj36g

Looking at the reproduction, it also seems that TrailingAction is missing a hover state.

Version

v37.11.2

Browser

No response

@prichodko prichodko added the bug Something isn't working label Feb 7, 2025
Copy link
Contributor

github-actions bot commented Feb 7, 2025

Uh oh! @prichodko, 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.

@langermank
Copy link
Contributor

Hey @prichodko, are you seeing this bug in production or have you only tested this code locally? We have a feature flag staff shipped right now called primer_react_css_modules_staff that fixes this issue. Once that FF ships it will be fixed by default, but that is likely a few weeks away.

Could you try enabling that locally to see if the problem persists? We may need to do some CSS overrides to make it look right for production depending on how quickly you need this.

@prichodko
Copy link
Author

prichodko commented Feb 13, 2025

@langermank – sorry for taking a while to answer, I was figuring out my entitlements.

Could you help me understand how I can tell if the primer_react_css_modules_staff FF is applied to my local environment?

I ran

bin/toggle-feature-flag enable primer_react_css_modules_staff

and noticed it is also enabled in the staff bar features dialog:

Image

I tried disabling it in that dialog, but I still don't see any difference. I know this might be a general question for FFs, but I'm still figuring out how it works. 😅

This comment has been minimized.

@prichodko
Copy link
Author

prichodko commented Feb 18, 2025

@langermank, I enabled the the FF locally and result is still the same:

github copilot layout issue

The workaround is implemented here https://github.com/github/github/pull/361173/files.

@langermank
Copy link
Contributor

Hey @prichodko I pulled your branch and refactored it to use ActionList.TrailingAction. With our feature flag enabled, it looks correct (as expected). I'm not sure why it looks broken for you, the steps you listed to enable it look correct.

ActionList with trailing action

That said, I think I would recommend not using it until this FF ships unless this particular feature is only staff shipped today. We're just a few weeks away. I'm happy to help refactor it later once TrailingAction looks correct!

@prichodko
Copy link
Author

@langermank – thanks for double-checking. I joined two weeks ago and am still learning my way around. I'd love to know what I'm doing wrong. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working react
Projects
None yet
Development

No branches or pull requests

2 participants