Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(demos): Too much space between toolbar title and left icon. #2174

Merged
merged 24 commits into from
Feb 14, 2018
Merged

fix(demos): Too much space between toolbar title and left icon. #2174

merged 24 commits into from
Feb 14, 2018

Conversation

touficbatache
Copy link
Contributor

There's too much space between the toolbar title and left icon. This PR fixes it.

demo

@touficbatache
fix(demos): Too much space between toolbar title and left icon.
2c21d8b 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
35fcd55 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
83f71f3 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
a269519 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
9f862de 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
e113916 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
fa66363 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
de14f7e 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
e23fc01 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
0b33ac1 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
777dd7d 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
a779f84 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
e16ac8b 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
2e6f40e 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
8b62459 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
c57b06b 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
00ba0b3 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
98e30f2 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
6ab4989 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
ff9d773 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
8461d65 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
32b4c3d 
Remove unnecessary styles and make demo page look better.
@touficbatache
fix(demos): Too much space between toolbar title and left icon.
9a82489 
Remove unnecessary styles and make demo page look better.
@codecov-io
Copy link

codecov-io commented Feb 2, 2018 •
edited
Loading

Codecov Report

Merging #2174 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2174   +/-   ##
=======================================
  Coverage   99.13%   99.13%           
=======================================
  Files          90       90           
  Lines        3830     3830           
  Branches      496      496           
=======================================
  Hits         3797     3797           
  Misses         33       33

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24b82dc...d12bcbb. Read the comment docs.

williamernest
williamernest reviewed Feb 14, 2018
View reviewed changes
Copy link
Contributor

@williamernest williamernest left a comment

Choose a reason for hiding this comment

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

I don't think we want to remove that element/class from each demo page. I think the same thing can be acheived by removing the padding-right from the catalog-back class and adding it back to the catalog-logo class in common.scss line 50

@williamernest williamernest self-assigned this Feb 14, 2018
@touficbatache
Copy link
Contributor Author

I didn't remove it @williamernest take a look at the code.

williamernest
williamernest reviewed Feb 14, 2018
View reviewed changes
Copy link
Contributor

@williamernest williamernest left a comment

Choose a reason for hiding this comment

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

The span was removed from the demo pages.

demos/common.scss
@@ -79,7 +79,6 @@ fieldset {

.catalog-logo,
.catalog-back {
padding-right: 24px;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep the padding-right property for the catalog-logo class.

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 we should... There is too much space between the logo and the title...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah wait, there is already a padding-right:24px; applied to the remaining class

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, not needed anymore.

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 why I removed it 😃

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, no longer needed.

demos/button.html
@@ -74,9 +74,7 @@
<header class="mdc-toolbar mdc-toolbar--fixed">
<div class="mdc-toolbar__row">
<section class="mdc-toolbar__section mdc-toolbar__section--align-start">
<span class="catalog-back">
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't be removing this span to change the padding-right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not removing the span? I kept the class 😃

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's not needed anymore :)

williamernest
williamernest approved these changes Feb 14, 2018
View reviewed changes
Copy link
Contributor

@williamernest williamernest left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

demos/button.html
@@ -74,9 +74,7 @@
<header class="mdc-toolbar mdc-toolbar--fixed">
<div class="mdc-toolbar__row">
<section class="mdc-toolbar__section mdc-toolbar__section--align-start">
<span class="catalog-back">
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's not needed anymore :)

demos/common.scss
@@ -79,7 +79,6 @@ fieldset {

.catalog-logo,
.catalog-back {
padding-right: 24px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, no longer needed.

@williamernest williamernest merged commit 3b0977d into material-components:master Feb 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@williamernest williamernest williamernest approved these changes

Assignees

@williamernest williamernest

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@touficbatache @codecov-io @williamernest