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

2.0 preview #103

Merged
merged 28 commits into from
May 12, 2017
Merged

2.0 preview #103

merged 28 commits into from
May 12, 2017

Conversation

notwaldorf
Copy link
Contributor

@notwaldorf notwaldorf commented Sep 12, 2016 •
edited
Loading

  • Deleted the deprecated /deep/ files
  • Mostly updated the @apply syntax

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

bicknellr
bicknellr reviewed Apr 5, 2017
View reviewed changes
bower.json Outdated
},
"devDependencies": {
"iron-component-page": "polymerelements/iron-component-page#^1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is iron-component-page ready to be used in 2.0-preview branches?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yas, it needs to come back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it came back

demo/index.html Outdated
min-height: 20px;
}

.main-container {
Copy link
Contributor

Choose a reason for hiding this comment

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

There doesn't seem to be any element with the class main-container here.

iron-flex-layout-classes.html Outdated
@@ -66,12 +66,6 @@
flex-wrap: wrap;
}

.layout.no-wrap {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this rule gone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it got merged to master after this PR was started. Will rebase :)

@notwaldorf
Copy link
Contributor Author

@bicknellr done! 🎉

bicknellr
bicknellr reviewed Apr 6, 2017
View reviewed changes
iron-flex-layout-classes.html
@@ -370,7 +370,6 @@
display: block;
}

/* IE 10 support for HTML5 hidden attr */
[hidden] {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're dropping support for IE 10, can we remove this ruleset altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think it's still useful -- remember, [hidden] is a lie, and if you don't define this it will never be applied correctly once a display:xxx has been set anywhere else

bicknellr
bicknellr approved these changes Apr 6, 2017
View reviewed changes
Copy link
Contributor

@bicknellr bicknellr left a comment

Choose a reason for hiding this comment

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

Other than this question about hidden and the known Chrome 57 bug causing tests to fail, I think this looks good.

@notwaldorf
Copy link
Contributor Author

Uhhh since when are the tests red

@bicknellr
Copy link
Contributor

btw GUIDE.md has a lot of @apply(...);

e111077
e111077 requested changes Apr 20, 2017
View reviewed changes
Copy link
Member

@e111077 e111077 left a comment

Choose a reason for hiding this comment

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

guide.md has quite a few @apply(...);

@notwaldorf notwaldorf merged commit 7157eee into master May 12, 2017
@valdrinkoshi valdrinkoshi mentioned this pull request Oct 27, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bicknellr bicknellr bicknellr approved these changes

@e111077 e111077 e111077 approved these changes

Assignees
No one assigned
Labels
cla: no
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@notwaldorf @googlebot @bicknellr @e111077 @tedium-bot @alvarezguille @bartolom