Fixed styling issues on logs page and made it more responsive

[floreks]

Fixes #467

I've changed logs toolbar on smaller screens to menu bar in order to fit it in available space.

There is 1 major issue with md-menu-bar that I've had to solve using !important property in css.

They've solved this issue: angular/material#6049 by changing style of parent md-toolbar to position: relative in here: https://github.com/angular/material/blob/b58343c20ac4bd3418629c29abddfe3ac3840eb5/src/components/menuBar/js/menuBarController.js#L90

This is changed by code so pure css is not enough to handle this better and in our case we need toolbar to have position: fixed that's why I've added !important.

Current view:

@bryk what do you think?

Btw. I'm out of office tomorrow and on monday so any comments I'll resolve on tuesday.

[codecov-io]

Current coverage is 83.59%

Merging #491 into master will not affect coverage as of 664bddd

@@            master    #491   diff @@
======================================
  Files           84      84       
  Stmts          695     695       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit            581     581       
  Partial          0       0       
  Missed         114     114       

Review entire Coverage Diff as of 664bddd

Powered by Codecov. Updated on successful CI builds.

[bryk]

Unfortunately, this does not fix the problem entirely:

[bryk]

Re "logs menu": how about changing this name to something more descriptive. I.e., users already see that this is a "menu" so this information is not needed. How about "logs source"?

[floreks]

Unfortunately, this does not fix the problem entirely:

On which browser did you check? I've checked on 3 different browsers (IE, Chrome, FF) and on chrome in mobile mode and it looks good. There's no wrapping. As soon as it wants to wrap, view changes to menu mode.

Re "logs menu": how about changing this name to something more descriptive. I.e., users already see that this is a "menu" so this information is not needed. How about "logs source"?

Sure. I've just used first name that came to my mind. "Logs source" sound better.

[floreks]

Ok I know where the problem is. Pod and container names may be very long and width of container is extended. I need to make sure that it changes to 'menu style' as soon as width extends some limit.

[floreks]

I've dealt with it differently.

PTAL

[bryk]

@maciaszczykm Can you take this?

[maciaszczykm]

Yes, I will do it soon.

[maciaszczykm]

Seems to be working as expected. Cannot reproduce @bryk's problem on Chrome, Firefox nor IE. Maybe it's due to operatin system?

My only concern is styling :)

• Could you move pickers slightly down? It's like 2 or 3 pixels to align it with Kubernetes logo.
• Could you make arrow downwards a bit smaller? It seems to big at the moment.
• Perhaps application and container names should not be transfromed to uppercase? I know, that's Angular Material's default, but you should consider it. Not neccessary however.

[floreks]

PTAL

[maciaszczykm]

---

Reviewed 2 of 4 files at r1, 1 of 2 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[bryk]

Thanks for doing this :)

[bryk]

Re position: fixed !important;: I'm pretty persistent on this and the opinion that we should never ever use it :) There is always a way to avoid !important unless you write a framework. Can we have this fixed and no-important rule reenabled? I'm happy to discuss this

[floreks]

I couldn't find other way to overload css that is set by the code. We would need our own code that overloads already overloaded css :) This seemed like too much to me, that's why I've decided to use !important. What about enabling no-importantrule and only disabling it in css with comment? If you have any better solution for that then it would be great.

[bryk]

What about enabling no-importantrule and only disabling it in css with comment?

This sounds very nice then. Say why we need important in this very place and enable the lint rule then :)

[floreks]

Unfortunately this feature for sass-lint is still under development: sasstools/sass-lint#70

We'd have to migrate to scss-lint to have option to disable particular css rule check. What do you think?

[bryk]

What problem does the !important fix, btw? I've tested it now, and !important does not change the behavior and no other rule wants to override position.

[floreks]

Remove !important then go to logs page. You need more logs so the page can be scrolled down. Click logs menu button. Toolbar will disappear because style will be changed by code to relative :)

[bryk]

Now I can reproduce this. I see that this is the menu bar that overrides the position: https://github.com/angular/material/blame/b58343c20ac4bd3418629c29abddfe3ac3840eb5/src/components/menuBar/js/menuBarController.js

I also see that single menus dont nee menu bars. How about removing the menu bar directive? https://material.angularjs.org/latest/demo/menu

[floreks]

That would require making 2 separate menus right next to each other. I'm not sure if they would fit in the available space for small devices.

[bryk]

Hmm... I see and dont like this :) We have an issue logged, so let's move forward and see that maybe material design fixes this.