-
-
Notifications
You must be signed in to change notification settings - Fork 333
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 grid list #739
Update grid list #739
Conversation
* Use camelCase attribute names * Use compassable component api. * Use composability mixins * Add basic render tests * Remove demo grid-list warning
Awesome |
{{#paper-grid-tile class="blue" md-rowspan="2"}} | ||
{{#paper-grid-tile-footer}} | ||
{{#paper-grid-list | ||
cols-sm="1" cols-md="2" cols-gt-md="6" |
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.
Did you keep dash-cased attributes here on purpose? It is true that size prefixes (-sm
, -xs
, -lg
, etc) usually are dash-cased. However, I think consistency wins here. Trying to think of a good api for this.
Let me know what you think.
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.
Maybe we could take inspiration from https://flexi.readme.io/docs/attributes
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.
Yes, mentioned it in the PR note.
Looks to me that flexi's api is essentially what we have now except we can't use simple names like sm
and md
as we have to differentiate between 3 properties with media options instead of flexi's 1.
Only reasonable options I can think of are:
a) cols="1"
cols-gt-xs="2"
b) cols="1"
colsGtXs="2"
c) cols="1 gt-xs-2"
Happy with any of these
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 find colsGtXs
very hard to read
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.
@Subtletree how hard could it be to implement c) ?
* Now uses attrs like `cols=“1 gt-xs-2 gt-md-4”`. * Re worked internals, using computed properties and `didUpdateAttrs` instead of observers. * Fixed `undefined` error messages when tile `colspans` weren’t compatible with grid `cols`. * Fixed `removeListener` to properly remove event listeners. * Should be some performance increase as it no longer re calculates everything all the time.
Ok ready for another review:
|
Think I know what's causing these errors, update soon. |
Passes locally 😕 |
This appears to be hitting the qunit problem in #694 also. @bjornharrtell Looks like you've fixed it in #747? Hopefully once that is merged this will pass. |
Thanks a lot for this, @Subtletree ! |
Attribute names are now camelCased e.g
rowHeight
. However the media suffixes are still dasherizedrowHeight-gt-xs
. Thought this would be inline with theflex-gt-xs
class names and easier to read imo thanrowHeightGtXs
. Let me know if you want it all camelCase though.Completes #738
p.s sorry about 'remote-tracking branch' commits