-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Card] Revert change to footer button alignment, add new footerActionAlignment prop #2407
Conversation
💦 Potential splash zone of changes introduced to
DetailsAll files potentially affected (total: 4)📄
|
4307921
to
66f64a6
Compare
8322d6c
to
fbf4738
Compare
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.
👍 Thanks for this! 👏
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.
LGTM 👍 🔥
e2d43bb
to
1d2305a
Compare
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.
58df5e2
to
80500b9
Compare
Hi all, I chatted with @dpersing who recommended we actually change DOM order in this case A good example was for partially sighted users who would:
I've updated the PR to change the DOM order as a result (previously it was CSS flex to change visual order) |
f0f9763
to
082fa90
Compare
…Alignment prop (left | right), default to right for backwards compat
082fa90
to
0cf759c
Compare
@@ -17,10 +18,10 @@ | |||
- Fixed an issue where the dropzone component jumped from an extra-large layout to a layout based on the width of its container ([#2412](https://github.com/Shopify/polaris-react/pull/2412)) | |||
- Fixed an issue which caused HSL colors to not display in Edge ([#2418](https://github.com/Shopify/polaris-react/pull/2418)) | |||
- Changed Button's `disclosure` prop to be `boolean | "up" | "down"`, allowing greater control over the direction the disclosure caret faces ([#2431](https://github.com/Shopify/polaris-react/pull/2431)) | |||
- Fixed an issue where the dropzone component jumped from an extra-large layout to a layout based on the width of it's container ([#2412](https://github.com/Shopify/polaris-react/pull/2412)) |
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.
There's a dupe of this above
Reverts #2104
See #2075 for more context on this change
WHY are these changes introduced?
It seemed to be generally accepted that this change is rendering buttons strangely, as seen in #2075 (comment).
After the PR I am reverting, polaris-react renders secondary buttons on the left, then primary buttons, which is contrary to the expected
Primary > Secondary
reading order. The blue button should be on the left.So, I set out to revert this change (because every consumer of
Card
would be expecting right-aligned buttons prior to that change, IMO that was a breaking change), and give the ability to control alignment.WHAT is this pull request doing?
Final product:
footerActionAlignment
prop to<Card>
to control the overall left/right alignment. Default toright
for historical consumersHow to 🎩
Copy-paste this code in
playground/Playground.tsx
:🎩 checklist
README.md
with documentation changes