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

fix(layout-grid): import the variables in the mixin #1232

Merged
merged 7 commits into from
Oct 24, 2017
Merged

fix(layout-grid): import the variables in the mixin #1232

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8954f51
fix(layout-grid): import the variables in the mixin
Sep 2, 2017
15be7c5
Improve the wordings of a sentence
Sep 5, 2017
c3db868
style(layout-grid): incorporate new grammatical suggestions to a sent…
Sep 6, 2017
02f9440
style(layout-grid): add missing semicolon
Sep 6, 2017
a150492
Merge branch 'master' into layout-grid-mixin-fix
kfranqueiro Oct 24, 2017
f9679cc
WIP Fix mistake during merge conflict resolution
kfranqueiro Oct 24, 2017
8ad94cc
WIP Fix whitespace introduced by GitHub...?
kfranqueiro Oct 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/mdc-layout-grid/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ When your contents need extra structure that cannot be supported by single layou

The nested layout grid behaves exactly like when they are not nested, e.g, they have 12 columns on desktop, 8 columns on tablet and 4 columns on phone. They also use the **same gutter size** as their parents, but margins are not re-introduced since they are living within another cell.

However, Material guideline do not recommend have a deeply nested grid since it might means a over complicated UX.
However, Material guideline does not recommend to have a deeply nested grid as it might mean a over complicated UX.
Copy link
Contributor

@touficbatache touficbatache Sep 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

That might be better 😃: "However, the Material Design guideline does not recommend having a deeply nested grid as it might mean an over complicated UX."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah seems better.


```html
<div class="mdc-layout-grid">
Expand Down
2 changes: 2 additions & 0 deletions packages/mdc-layout-grid/_mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

@import "./variables"

// returns the lower grid boundary or null if the smallest grid is selected
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the linter is complaining here

17:2 ✖ Expected indentation of 2 spaces indentation

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 don't think I get that. What should be fixed?

Copy link
Contributor

Choose a reason for hiding this comment

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

You should indent the line 17 by 2 spaces. Not more, not less.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's surprising I thought it was correctly indented after seeing it. Moreover, I didn't touch that line at all in the first place.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe someone edited it in the past but it got accepted even with the error

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 can't take this. This is so odd. Why doesn't the linter warn about a missing indentation at line 27 or line 39 which has a similar comment to this line?

Copy link
Contributor

Choose a reason for hiding this comment

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

Your text editor may have parsed and saved it incorrectly. You can always run npm run lint to see what will run on TravisCI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I found the issue. I missed a semicolon on line 15.

@function mdc-layout-grid-breakpoint-min($size) {
@if not map-has-key($mdc-layout-grid-columns, $size) {
Expand Down