-
Notifications
You must be signed in to change notification settings - Fork 4
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
refactor(Popover)!: remove showArrow from popover #1461
Conversation
Codecov Report
@@ Coverage Diff @@
## next #1461 +/- ##
==========================================
- Coverage 91.74% 91.73% -0.01%
==========================================
Files 293 293
Lines 4359 4355 -4
Branches 779 777 -2
==========================================
- Hits 3999 3995 -4
Misses 333 333
Partials 27 27
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
children: ( | ||
<> | ||
<Popover.Button as={Button}>Open Popover</Popover.Button> | ||
<Popover.Content showArrow> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the showArrow
prop be removed as well?
Edit: actually you're probably keeping that prop for functionality until it is completely removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I think it would be ok to axe the showArrow
prop and the couple usages in the recipes, since it's not being used downstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's fair. in fact, our 8.0 will mostly be trimming away components/props, so we can capture that in the next release and avoid creating a 9.0 JUST for this prop removal
951360c
to
f824514
Compare
Summary:
showArrow
propTest Plan: