-
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 bricks #39
Refactor bricks #39
Conversation
91133df
to
b592557
Compare
d800734
to
79f1fbf
Compare
6b7ba67
to
8394128
Compare
nypr card refactors
this will be resurrected as brick-row-blowout
layout, formatting, and use of component attributes also allow for yielding a block
so it can be used as-is in the template on other elements
26672a4
to
e48837a
Compare
export default Ember.Component.extend({ | ||
layout, | ||
classNames: ['row__col'], | ||
classNameBindings: ['flipped'] |
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 like there may be a small attribute name inconsistency between flip vs. flipped - on this component, if you pass flipped=true
it would get flipped, but down on nypr-brick-row-blowout.hbs
(https://github.com/nypublicradio/nypr-ui/pull/39/files#diff-6141847a1a5efc27761d45f7bd66729fR9), it checks for flip=true
.
I think if this line was written as classNameBindings: ['flip:flipped']
they would both simply require flip=true
Refactors
nypr-brick-layout
to expose a few levels of contextual components, which should make it easier for upstream apps to arbitrarily replace and re order parts of the layout.One thing worth noting is that this renames
nypr-brick-image-row
tonypr-brick-row-blowout
, so it has a naming that's more consistent with the convention we've set up.