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

feat(assignee-selector): Create storybook component for new assignee selector trigger #70561

Merged
merged 19 commits into from
May 10, 2024

Conversation

MichaelSun48
Copy link
Member

@MichaelSun48 MichaelSun48 commented May 8, 2024

This PR creates a new component <AssigneeBadge/> which will eventually replace the current assignee selector trigger in the issue stream. This PR does not make any changes to the selector, it merely creates the component and a corresponding storybook entry so it can be iterated on independent of any changes made to the issue stream.

More details of the project can be found here (#69827)

At a glance (as of 5/9):
image

Note that unlike the previous assignee selector, there is no longer a "Suggested Assignee" state for this new assignee selector, there is only an Assigned and Unassigned state.

TODO:

  • Figure out what the loading state looks like
  • Chevron weight and style match border
  • Add tests

@MichaelSun48 MichaelSun48 changed the title feat(issues-stream): Create storybook component for new assignee selector trigger feat(assignee-selector): Create storybook component for new assignee selector trigger May 8, 2024
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 8, 2024
@MichaelSun48 MichaelSun48 requested a review from a team May 8, 2024 23:40
Comment on lines 92 to 97
// Team avatars need extra left padding since the square avatar
// is being fit into a rounded edge
css={css`
--avatar-spacing: ${space(0.5)} ${space(0.25)} ${space(0.5)}
${assignedTo.type === 'team' ? space(0.75) : space(0.5)};
`}
Copy link
Member Author

Choose a reason for hiding this comment

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

image image

Left image is what it looks like without the extra left padding. Right is with extra padding. imho it looks very cramped and needs the extra padding, but lmk if you don't think so

Copy link
Member

@roggenkemper roggenkemper left a comment

Choose a reason for hiding this comment

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

does this still work for the suspect commit suggested assignee?

@MichaelSun48
Copy link
Member Author

does this still work for the suspect commit suggested assignee?

@roggenkemper
If the issue has a suggested assignee because of a suspect commit, that suggestion will still be surfaced in the dropdown itself. However since there is no longer a suggested assignee state for this new trigger, we will no longer surface that information on hover like in this example from the current UI:

image

This is not 100% set in stone though, so we can discuss this further if you have strong opinions

@roggenkemper
Copy link
Member

maybe this is just me but i feel like the padding between the avatar and the name could be increased a bit

@MichaelSun48
Copy link
Member Author

Updated designs:
image

@MichaelSun48 MichaelSun48 requested a review from scttcper May 9, 2024 03:07
@MichaelSun48 MichaelSun48 merged commit d17225a into master May 10, 2024
43 of 44 checks passed
@MichaelSun48 MichaelSun48 deleted the msun/upgradeAssigneeSelectorTrigger branch May 10, 2024 20:55
Copy link

sentry-io bot commented May 10, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: Cannot read properties of undefined (reading 'id') /stories/ View Issue

Did you find this useful? React with a 👍 or 👎

@github-actions github-actions bot locked and limited conversation to collaborators May 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants