-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update page to v2.11.0 #218
Conversation
@@ -104,4 +115,11 @@ export default Component.extend({ | |||
hasActions: or('primaryAction', 'secondaryActions').readOnly(), | |||
hasSecondaryActions: gt('secondaryActions.length', 0).readOnly(), | |||
hasRollup: gt('secondaryActions.length', 1).readOnly(), | |||
|
|||
shouldRenderPrimaryActionAsPrimary: computed('primaryAction.primary', function() { |
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.
Very happy to rename this if anyone can think of anything better 😉
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.
Yeah, it's weird that this is a thing now but 🤷♂️ I'm good with it
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
@@ -104,4 +115,11 @@ export default Component.extend({ | |||
hasActions: or('primaryAction', 'secondaryActions').readOnly(), | |||
hasSecondaryActions: gt('secondaryActions.length', 0).readOnly(), | |||
hasRollup: gt('secondaryActions.length', 1).readOnly(), | |||
|
|||
shouldRenderPrimaryActionAsPrimary: computed('primaryAction.primary', function() { |
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.
Yeah, it's weird that this is a thing now but 🤷♂️ I'm good with it
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.
Looks good. Can we add a test for this?
@sivakumar-kailasam I wasn't going to because there aren't any tests for either of these features in the React implementation at the moment. Can add some if @vladucu / @tomnez think it's worthwhile though... |
Would be nice to drop in a quick test, nothing fancy, just that we render these properties correctly |
@sivakumar-kailasam @vladucu added a couple of tests, not run codemods on them though because it causes issues with registering the mock |
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.
👍
Closes #217 by adding
titleMetadata
andprimary
support to page header.Screen grab with
titleMetadata=(component "polaris-badge" status="success" text="Metadata")
andprimaryAction=(hash primary=false ...)
: