Material-components-web: Top margin of mdc-toolbar-fixed-adjust is incorrect when toolbar width between 600px and 960px.

Created on 4 Jul 2017  路  6Comments  路  Source: material-components/material-components-web

What MDC-Web Version are you using?

0.14.0

What browser(s) is this bug affecting?

Chrome 59.0.3071.115

What OS are you using?

Windows 7 Professional SP1

What are the steps to reproduce the bug?

Please see https://jsfiddle.net/r7nds0gs/
And resize window width from under 600px to over 960px.

What is the expected behavior?

Top of the main content sholud be adjusted to bottom of the toolbar.

What is the actual behavior?

When toolbar width is between 600px and 960px, top of the main content is hidden by toolbar.
Margin of the main content may be not enough, I guess.

Any other information you believe would be useful?

backlog bug

Most helpful comment

peek 2018-02-13 14-20

All 6 comments

This could be fixed by changing this:

@media (max-width: 959px) and (orientation: landscape){
  .mdc-toolbar-fixed-adjust {
      margin-top: 48px;
  }
}

to that:

@media (max-width: 959px) and (orientation: landscape){
  .mdc-toolbar-fixed-adjust {
      margin-top: 56px;
  }
}

Hmmmm.... The problem with that fix is that it will make the toolbar too tall for tablets in landscape.

Willing to review any PR's, but I think we might need more specificity on the tablet media query.

The headroom of .mdc-toolbar-fixed-adjust assumes that the height of the .mdc-toolbar__row is controlled by it's min-height.

The media queries applies correctly to both .mdc-toolbar__row and .mdc-toolbar-fixed-adjust. The issue is that the height of .mdc-toolbar is influenced by it's content height (app title, icons).

One way to address it is to apply the same media query to adjust vertical paddings of .mdc-toolbar__icon and .mdc-toolbar__title to 8px (top & bottom). This will, however, make the toolbar look tighter in the targeted media (<= 959px & landscape) (mock below). But I think it's better than trying to scale both padding and element size (a lot more coordination and significantly increases complexity).

screen shot 2017-09-30 at 9 18 09 am

If that's desirable, I can start working on a PR. I can also propose a refactor that group this piece of vertical layout logic in a more coherent way鈥攈opefully reduce chance of this confusion in the future.

peek 2018-02-13 14-20

Tracking this since it's clearly still not fully resolved, and we're currently working on revamping toolbar in the context of top app bar use cases.

screenshots here https://github.com/material-components/material-components-web/issues/1841

also

The existing MDCToolbar component and styles will be removed in a future release. Some of its functionality will be available in the mdc-top-app-bar package instead. Bugs and feature requests will no longer be accepted for the mdc-toolbar package. It is recommended that you migrate to the mdc-top-app-bar package to continue to receive new features and updates.

https://github.com/material-components/material-components-web/tree/master/packages/mdc-toolbar

Was this page helpful?
0 / 5 - 0 ratings

Related issues

traviskaufman picture traviskaufman  路  3Comments

abhiomkar picture abhiomkar  路  3Comments

AbuMareBear picture AbuMareBear  路  3Comments

ronnieroyston picture ronnieroyston  路  3Comments

traviskaufman picture traviskaufman  路  3Comments