-
Notifications
You must be signed in to change notification settings - Fork 7
Layout's annotated section to use a new css selector instead of subdued text style for description #191
Conversation
CHANGELOG.md
Outdated
@@ -4,6 +4,7 @@ | |||
- [#168](https://github.com/smile-io/ember-polaris/pull/168) [INTERNAL] Upgrade to Ember CLI `3.4.2` | |||
- [#171](https://github.com/smile-io/ember-polaris/pull/171) [UPDATE] Shopify Polaris `v2.11.0` | |||
- [#174](https://github.com/smile-io/ember-polaris/pull/174) [FEATURE] Add `polaris-inline-error` component | |||
- [#191](https://github.com/smile-io/ember-polaris/pull/191) [ENHANCEMENT] Layout's annotated section to use a new css selector instead of subdued text style for description |
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.
Nit: this description is pretty complex for those just wanting to have an overview of what's happened. How about something like Update
polaris-layout/annotated-section styling to match Polaris v2.11.0
instead?
@@ -22,6 +22,13 @@ const layoutAnnotationWrapperSelector = buildNestedSelector( | |||
'div.Polaris-Layout__AnnotationWrapper' | |||
); | |||
|
|||
const annotationSectionDescriptionSelector = (containerSelector) => |
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.
Nit: the name annotationSectionDescriptionSelector
makes this sound like an actual selector, rather than a function that generates a selector.
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.
Would converting this file/component to use the new test selectors help here?
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.
ah yes, didn't realize we had that here already 🙂
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.
I reckon this looks good now, thanks @sivakumar-kailasam! 👍
Wide screen
** Small screen **
Fixes #190