Skip to content

Support ReactNode in ActionMenu.Overlay - #2168

Merged
siddharthkp merged 4 commits into
primer:support-reactnode-in-actionmenufrom
jdrush89:support-reactnode-in-actionmenu
Jul 26, 2022
Merged

Support ReactNode in ActionMenu.Overlay#2168
siddharthkp merged 4 commits into
primer:support-reactnode-in-actionmenufrom
jdrush89:support-reactnode-in-actionmenu

Conversation

@jdrush89

@jdrush89jdrush89 commented Jul 12, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Describe your changes here.

Add support for ReactNode as the child type in ActionMenu.Overlay so that conditional children can be rendered within an overlay.
Closes#2067

Screenshots

No visual changes

Merge checklist

  • Added/updated tests - current testing doesn't seem granular enough to warrant testing every possible prop type, I can add a test if it seems necessary though.
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@jdrush89
jdrush89 requested review from a team and siddharthkpJuly 12, 2022 01:53
@changeset-bot

changeset-botBot commented Jul 12, 2022•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a16d79c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jdrush89jdrush89 changed the title Support reactnode in actionmenuSupport reactnode in ActionMenu.OverlayJul 12, 2022
@jdrush89jdrush89 changed the title Support reactnode in ActionMenu.OverlaySupport ReactNode in ActionMenu.OverlayJul 12, 2022
@siddharthkp

siddharthkp commented Jul 20, 2022

Copy link
Copy Markdown
Member

This looks pretty safe, can we test this with a canary build in memex just to be sure?

@jdrush89

jdrush89 commented Jul 26, 2022

Copy link
Copy Markdown
ContributorAuthor

@siddharthkp sure, what is the process for that?

@siddharthkp

siddharthkp commented Jul 26, 2022•
edited
Loading

Copy link
Copy Markdown
Member

Ugh, canary release workflow was skipped on fork. Let's merge this into a branch in primer/react and continue from there.

Update: Creating a new pull request here: #2188 and invited you to the primer/react repository

Published @primer/react@0.0.0-202262614251

@siddharthkp
siddharthkp changed the base branch from main to support-reactnode-in-actionmenuJuly 26, 2022 14:15
@siddharthkp
siddharthkp merged commit 49fe987 into primer:support-reactnode-in-actionmenuJul 26, 2022
@siddharthkpsiddharthkp mentioned this pull request Jul 26, 2022
Merged
6 tasks
colebemis added a commit that referenced this pull request Aug 25, 2022
@siddharthkp@jdrush89@colebemis
Support ReactNode in ActionMenu.Overlay (#2168) (#2188)
8fc2e42
* Supporting ReactNode type for ActionMenu children
* Fixing spacing
* Updating ActionMenu.Overlay props docs
* Adding changeset
Co-authored-by: Joshua Rush <jdrush89@gmail.com>
Co-authored-by: Cole Bemis <colebemis@github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@siddharthkpsiddharthkpAwaiting requested review from siddharthkp

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ActionMenu.Overlay should accept false and null as children

2 participants

@jdrush89@siddharthkp