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

PR View: Popover Dropdown component #15339

Merged
merged 2 commits into from
Sep 28, 2022

Conversation

tidy-dev
Copy link
Contributor

Based on #15326

Description

This PR separates out the popover dropdown logic of the branch select component into a new component that I can reuse for the commit list popover.

I also updated the styles to match the figma better.. however, based on our accessibility meeting recently, I figured the link look would be recommended to be underlined to get the required contrast to be accessible, so I added that. But, now that I see it, I think that the design of this component should be revisited. Maybe since this is a button, it should look like a button. It looks like a button on dotcom (when editable) and we do have a dropdown button to context menu elsewhere in our app (repository add button). But, that I will leave to another PR, this PR is to add the dropdown to popover functionality so I can add the commit selection dropdown.

cc: @gavinmn

Screenshots

Screen.Recording.2022-09-22.at.11.49.55.AM.mov

Release notes

Notes: no-notes

@gavinmn
Copy link
Contributor

gavinmn commented Sep 22, 2022

Looking good.

But, now that I see it, I think that the design of this component should be revisited. Maybe since this is a button, it should look like a button. It looks like a button on dotcom (when editable) and we do have a dropdown button to context menu elsewhere in our app (repository add button).

I agree that we should change this and we can introduce it in a separate PR.

@sergiou87 sergiou87 self-assigned this Sep 28, 2022
Copy link
Member

@sergiou87 sergiou87 left a comment

Choose a reason for hiding this comment

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

Looks great! 🤩 :shipit:

@tidy-dev tidy-dev merged commit aff0554 into pr-files-changed-header Sep 28, 2022
@tidy-dev tidy-dev deleted the dropdown-popover-component branch September 28, 2022 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants