mozilla / shielddash

Dashboard for SHIELD study reports.
http://shielddash.herokuapp.com/
Mozilla Public License 2.0
1 stars 5 forks source link

add percentages toggle on study details page #28

Closed spasovski closed 8 years ago

spasovski commented 8 years ago

This addresses issue #13

openjck commented 8 years ago

Just one small question about the code. Are there any places where we could un-nest selectors without affecting the end result? For example, do we need .study-wrapper.show-percentage .study-results-row b.count or would b.count suffice?

This case might be fine. Just double-checking because it can be a tricky balance. Nesting definitely makes the code more readable, but sometimes causes the selectors to be overly specific and long.

spasovski commented 8 years ago

The CSS could do with some nesting cleanup for sure before it gets out of hand. I have a styling cleanup branch locally that addressed some other issues but I'll bundle that there too. For this PR I'll look at the other concerns and get the next PR ready after.

openjck commented 8 years ago

Looks beautiful. Love it!