Skip to content
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

[GridList] - new component #1320

Merged
merged 7 commits into from
Oct 5, 2015
Merged

[GridList] - new component #1320

merged 7 commits into from
Oct 5, 2015

Conversation

igorbt
Copy link
Contributor

@igorbt igorbt commented Aug 2, 2015

Simple flex-box based Grid List implementation. Support tiles with arbitrary cell size, but cannot implement complex layouts (like Angular Material GridList), is limited to flex-box limitations.
gridlist

@jkruder jkruder mentioned this pull request Aug 7, 2015
@oiwn
Copy link

oiwn commented Sep 8, 2015

Any news?

@igorbt
Copy link
Contributor Author

igorbt commented Sep 8, 2015

As you can see, no core contrubutor/owner did comment on this PR. So, I suppose there's little interest in this component. I use it in my own app and I also did a Virtual Grid List for the huge number of items use case. But I cannot do a PR with it until this one is not merged.

@oliviertassinari
Copy link
Member

this PR can't be merge, you will have to rebase

@chrisfls
Copy link

I'll be using this (local copy while this is not merged).

@shaurya947
Copy link
Contributor

@igorbt please rebase when you get a chance so we can go ahead and merge this :)

@oiwn
Copy link

oiwn commented Sep 29, 2015

wow, very good! Thank you.

@igorbt
Copy link
Contributor Author

igorbt commented Sep 30, 2015

OK, I think I can find some time to rebase this week. That's a pretty old PR so I think some refactoring may be needed.

@shaurya947
Copy link
Contributor

@igorbt looking forward to it!

Simple flex-box based Grid List (https://www.google.com/design/spec/components/grid-lists.html) implementation. Support only tiles with 1x1 cell size.
After 0.12.0 breaking changes for theming, spacing and components themes are accessed in a different way.
@igorbt
Copy link
Contributor Author

igorbt commented Oct 2, 2015

OK, I did the rebase. Didn't invest too much time in a refactoring, just updated the way theming variables are accessed. So, I'm open to code improvements suggestions if any.

@@ -0,0 +1,68 @@
let React = require('react/addons');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need addons here?

Made both components to be consistent with other components in how muiTheme is handled.
@igorbt
Copy link
Contributor Author

igorbt commented Oct 5, 2015

@oliviertassinari, you were right, I didn't need react/addons there. After all, I didn't use PureRenderMixin from the addons as I intended.

Meanwhile I saw that I needed to refactor the way muiTheme is handled, I saw that all components now put it in the state. While I see the benefit of it, IMHO this adds a bit of boilerplate code to all the components (I basically copy-pasted ~20 lines from other component and I HATE copy-pasting).

@shaurya947
Copy link
Contributor

I'll go ahead and merge this. We can take any issues that come along the way :-)

shaurya947 added a commit that referenced this pull request Oct 5, 2015
[GridList] - new component
@shaurya947 shaurya947 merged commit 972e0c7 into mui:master Oct 5, 2015
@igorbt igorbt deleted the gridlist branch October 20, 2015 03:23
@zannager zannager added the component: Grid The React component. label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Grid The React component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants