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.

Sign up for GitHub

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

Jump to bottom

[List] Finish the initial PR #6367

Merged
merged 2 commits into from
Mar 18, 2017
Merged

[List] Finish the initial PR #6367

merged 2 commits into from
Mar 18, 2017

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Mar 17, 2017 •
edited
Loading

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

This is a continuation of #6316.

@oliviertassinari oliviertassinari changed the title Next list advanced [List] Finish the initial PR Mar 17, 2017
@oliviertassinari oliviertassinari mentioned this pull request Mar 17, 2017
Closed
15 tasks
@oliviertassinari oliviertassinari added component: list This is the name of the generic UI component, not the React module! next labels Mar 17, 2017
@muibot muibot added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI PR: Needs Review and removed PR: Needs Review labels Mar 17, 2017
@oliviertassinari oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Mar 17, 2017
@oliviertassinari
Copy link
Member Author

The coverage is falling because of the code branch for the warning. I don't want to test that.

@muibot muibot added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI PR: Needs Review and removed PR: Needs Review labels Mar 17, 2017
@oliviertassinari oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Mar 17, 2017
@oliviertassinari
Copy link
Member Author

@kybarg Any feedback welcomed. I will merge it once you are happy with it.

@kybarg
Copy link
Contributor

kybarg commented Mar 18, 2017 •
edited
Loading

@oliviertassinari this PR looks fine to me 👍
Later I will open another PR to adjust two and three line lists.

@mbrookes
Copy link
Member

The icons are mostly still too dark (<IconButton>comment</IconButton>) is correct, but that icon still needs to be switched to an SvgIcon. Both mentioned on previous PR.

Otherwise, looks good to me.

As for the tear-effect: It will return victorious! 😄

@oliviertassinari
Copy link
Member Author

@kybarg @mbrookes Awesome, some iterations are still needed but we are moving forward 😄 .

@oliviertassinari oliviertassinari merged commit 95a7e53 into mui:next Mar 18, 2017
@oliviertassinari oliviertassinari deleted the next-list-advanced branch March 18, 2017 12:35
@muibot muibot added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Needs Review labels Mar 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
component: list This is the name of the generic UI component, not the React module! PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@oliviertassinari @kybarg @mbrookes @muibot