Refactor top-app-bar into top-app-bar, top-app-bar-fixed, and top-app… #379
Conversation
…-bar-short - [x] Remove fixed and short options from mwc-top-app-bar - [x] remove `centerTitle` from mwc-top-app-bar - [x] mwc-top-app-bar README - [ ] mwc-top-app-bar-fixed - [ ] mwc-top-app-bar-fixed README - [ ] mwc-top-app-bar-short - [ ] mwc-top-app-bar-short README
- Full README - Short variant has a modified MDC foundation to fix material-components/material-components-web#4928, upstreamed as material-components/material-components-web#5009
|
Cheating a bit by forking the foundation change I made in material-components/material-components-web#5009, but it was the only good way to fix the scrolling bug with the |
| */ | ||
|
|
||
| /* NOTE: Delete this after | ||
| * https://github.com/material-components/material-components-web/pull/5009 |
There was a problem hiding this comment.
Looks like this MDC PR was approved, so can probably remove before merging this PR? I guess we'd need a release though, what's the MDC release schedule?
Fix images in READMEs to just be the bar, 480px wide
|
|
||
| ### Prominent and Dense | ||
|
|
||
| <img src="images/prominent_and_dense.png" height="96px"> |
There was a problem hiding this comment.
The title cuts off in this screenshot.
There was a problem hiding this comment.
I think I can remove the Fixed part from these, and that should fix it.
|
|
||
| Fixed Top App Bars are a container for items such as application title, navigation icon, and action items that are always visible. | ||
|
|
||
|  |
There was a problem hiding this comment.
The gifs are awesome, exactly what's needed to show the difference between the three layouts. I can't help myself notice these nits though...
- It looks very blurry on my retina screen (whereas the other screenshots below it look perfect). Was it recorded on a low dpi screen?
- I think it'd look better without the mouse cursor visible (put the mouse lower down so it's not in the crop?)
- I think it'd look better if the text was left padded, and probably a lower font weight (e.g. like in this screenshot from the spec?)
There was a problem hiding this comment.
Didn't change the font weight, but a 16px padding left and right looks a lot better.
There was a problem hiding this comment.
Looks better! Still looks blurry though.
There was a problem hiding this comment.
Hmm, this is only looking blurry on a high DPI display. I think the tool I'm using is just not capturing at 2x? Or maybe this is as good as I can get from it.
There are alternate options, but maybe this is acceptable for now and I can look into improving the gif quality in the future?
There was a problem hiding this comment.
Yeah looks like licecap might be bad at doing high dpi: justinfrankel/licecap#9. Ok, if you don't want to spend more time on it, your call.
…-bar-short
centerTitlefrom mwc-top-app-barFixes #329
Fixes #351