734194 - Clean up style.css and undo generic rules such as table stuff

Status: RESOLVED FIXED

(Webtools Graveyard :: Elmo, defect, P1)

Description

[Peter Bengtsson [:peterbe]]

* Every table should define a class.

* Remove the the quick-search selectors

* Some "teams" specific selectors --> teams.css (audit for more like this)

* Review the use of the `*` selector

Comment 1

[Axel Hecht [:Pike]]

Marking this as P1, this should block merging dev to prod.

Comment 2

[Peter Bengtsson [:peterbe]]

Attachment: style.css cleaned up

No default table styling. 
License header on style.css
Introduced dedicated index.css for the home page. 
Cell borders look great on the dashboard.

Comment 3

[Axel Hecht [:Pike]]

Comment on attachment 607631 [details] [diff] [review]
style.css cleaned up

Review of attachment 607631 [details] [diff] [review]:
-----------------------------------------------------------------

r-, I think the rules are still bleeding back and forth, and most notably, the display:block rule is still too generic and has hacks to revert it.

Also, there seem to be a lot of rules that need to be put in for the exhibit table views. I'd rather have the rules they need to overwrite to be more specific than having css going back and forth.

::: apps/shipping/static/shipping/css/dashboard.css
@@ +134,1 @@
>  }

Many of these seem to be hacks against too generic rules elsewhere.

I'm also concerned about the descendant combinators for tr, td, th, can we make those explicit child selectors?

::: static/css/style.css
@@ +411,5 @@
>  /* Footer */
>  .push {
>      clear: both;
>      height: 75px;
>  }

This rule doesn't seem to belong here, I think. The only class="push" thing I find ad-hoc is in the signoffs view, so probaby this should be in signoffs.css?

@@ +467,3 @@
>      padding: 10px 7px;
>      vertical-align: top;
>  }

This sounds like another rule that would benefit from not having a descendent combinator. Also, I'm not sure that td padding is generally wanted.

@@ +471,2 @@
>      display: block;
>  }

This rule should go, and only ever be used in dedicated td's, I guess. We hack around that being here all over the place.

Where does this rule actually do what it should? I'm really confused why we'd use a span if we want a block display.

@@ +475,3 @@
>      text-align: center;
>      vertical-align: middle;
>  }

Where do these take effect? My attempts to find them failed so far.

Comment 4

[Schalk Neethling [:espressive]]

Comment on attachment 607631 [details] [diff] [review]
style.css cleaned up

Review of attachment 607631 [details] [diff] [review]:
-----------------------------------------------------------------

::: apps/shipping/static/shipping/css/dashboard.css
@@ +128,5 @@
> +    background-image:     -ms-linear-gradient(top, white, #eee);
> +    background-image:      -o-linear-gradient(top, white, #eee);
> +    background-image:         linear-gradient(top, white, #eee);
> +}
> +table.exhibit-tabularView-body td.bugs_cell span {

Here we are coding around the td span { display: block; } rule. If we got rid of that one, we can get rid of this one.

Comment 5

[Schalk Neethling [:espressive]]

Comment on attachment 607631 [details] [diff] [review]
style.css cleaned up

Review of attachment 607631 [details] [diff] [review]:
-----------------------------------------------------------------

::: static/css/style.css
@@ +411,5 @@
>  /* Footer */
>  .push {
>      clear: both;
>      height: 75px;
>  }

This was related to the way footer stick used to work but is no longer required. i believe we can get rid of this.

Comment 6

[Schalk Neethling [:espressive]]

Comment on attachment 607631 [details] [diff] [review]
style.css cleaned up

Review of attachment 607631 [details] [diff] [review]:
-----------------------------------------------------------------

::: static/css/style.css
@@ +475,3 @@
>      text-align: center;
>      vertical-align: middle;
>  }

I can also not find any reference to this in CSS as well as JS files.

Comment 7

[Peter Bengtsson [:peterbe]]

Attachment: stuff removed and stuff fixed

Thank you for your patience Axel!

It's getting a bit messy here in the bug but I hope I managed to address all issues. 

* I'm not going to touch on the preferred padding on `table.standard td`. It's not "wrong" and is part of the design rather. 

* With this patch, the sign-off view now looks great. The odd top-left & bottom-left rounded borders on the left-most column still doesn't work great. I chalk that up to being the wrong way entirely to do such a design. What we're getting now is at least really usable. http://cl.ly/0a0Y0f2s0U1l2y2S3Q1O

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 608880 [details] [diff] [review]
stuff removed and stuff fixed

Review of attachment 608880 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thanks for hanging in on this.

Comment 9

[Peter Bengtsson [:peterbe]]

https://github.com/mozilla/elmo/commit/8fad6ef8af70eb4be94fa768da05a8fd55669e12