Closed Bug 1193804 Opened 9 years ago Closed 9 years ago

break up treeherder.css into logical smaller files

Categories

(Tree Management :: Treeherder, defect, P3)

Product:
Tree Management
Component:
Treeherder
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: camd, Assigned: jfrench)

Details

Attachments

(8 files)

PR 939 - userguide
46 bytes, text/x-github-pull-request
camd
: review+
Details | Review
PR 945 - navbar
46 bytes, text/x-github-pull-request
camd
: review+
Details | Review
PR 953 - navbar-panels
46 bytes, text/x-github-pull-request
camd
: review+
Details | Review
PR 959 - job-buttons
46 bytes, text/x-github-pull-request
camd
: review+
Details | Review
PR 975 - pinboard
46 bytes, text/x-github-pull-request
camd
: review+
Details | Review
PR 985 - info-panel
46 bytes, text/x-github-pull-request
camd
: review+
Details | Review
PR 1000 - resultsets
47 bytes, text/x-github-pull-request
camd
: review+
Details | Review
PR 1008 - global
47 bytes, text/x-github-pull-request
camd
: review+
Details | Review 
we could likely break this up into sections like this (perhaps)

general/common identifiers
custom buttons
header (nav bar, watched repos bar stuff)
plugins (bottom) panel

perhaps even separate jobs from revisions?  Not sure...
or...
use your imagination.  :)
I'll take this, and do it at a time after we land job counts and don't have a lot of css changes underway.
Assignee: nobody → tojonmz
Status: NEW → ASSIGNED
Priority: -- → P3
Will mentioned it would be worthwhile to check to see if there's any grouping conventions, I haven't found anything definitive in searches so far. I did check Marketplace and it seems to tailor its groupings for its own needs.
https://github.com/mozilla/fireplace/tree/master/src/media/css

Proposing for treeherder we could have:
css/treeherder-global.css
css/treeherder-navbar.css
css/treeherder-menus.css
css/treeherder-resultsets.css
css/treeherder-buttons.css
css/treeherder-info-panel.css (includes pinboard, or we could split this out)
css/treeherder-filters.css
css/treeherder-result-status.css (includes our '-shading' color classes)
css/treeherder-help.css

And potentially bundle/minify the css during build(?) if we can quantify sufficient performance benefit.
There's a lot of css changes in camd's job grouping bug 1163064 and in my 'unknown jobs/steps' bug 1192917 which are both close to being done, so we'd like to land those first before I start splitting.
Ok, ready to go. I plan to do the change in separate PR's, one for each file. It should make it quick to land, easy to review, and prevent resolves.. vs. doing the entire break up over a longer period during which css changes may land.
Attached file PR 939 - userguide — 
PR for userguide for review.
Attachment #8656828 - Flags: review?(cdawson)
Comment on attachment 8656828 [details] [review]
PR 939 - userguide

lgtm.  :)
Attachment #8656828 - Flags: review?(cdawson) → review+
(In reply to Jonathan French (:jfrench) from comment #2)

> css/treeherder-filters.css

I will probably change this to '-panels' when I get to it, it will include the filter panel, the sheriff panel, and also any future ones we create. Our main Info Panel (aka. pluginpanel) will still get its own file.
Attached file PR 945 - navbar — 
PR for navbar for review.
Attachment #8657884 - Flags: review?(cdawson)
Attachment #8657884 - Flags: review?(cdawson) → review+
(In reply to Jonathan French (:jfrench) from comment #2)
> css/treeherder-filters.css

Going with css/treeherder-navbar-panels.css for this; it's more generalized and we will include the sheriff panel.
Attached file PR 953 - navbar-panels — 
PR for navbar-panels for review.
Attachment #8658822 - Flags: review?(cdawson)
Attachment #8658822 - Flags: review?(cdawson) → review+
> css/treeherder-buttons.css

Going with '-job-buttons' for this one (one of the main motivations for the splitting), and to differentiate from other buttons which will end up in other files.
Attached file PR 959 - job-buttons — 
PR for job-buttons for review.
Attachment #8660729 - Flags: review?(cdawson)
Attachment #8660729 - Flags: review?(cdawson) → review+
Adding a '-pinboard' to split it out separately from the info-panel.
Attached file PR 975 - pinboard — 
PR for -pinboard for review.
Attachment #8662965 - Flags: review?(cdawson)
Attachment #8662965 - Flags: review?(cdawson) → review+
Attachment #8662965 - Attachment description: PR 975 → PR 975 - pinboard
Attached file PR 985 - info-panel — 
PR for info-panel for review.
Attachment #8663800 - Flags: review?(cdawson)
Attachment #8663800 - Flags: review?(cdawson) → review+
Attached file PR 1000 - resultsets — 
PR for -resultsets for review.
Attachment #8664976 - Flags: review?(cdawson)
Comment on attachment 8664976 [details] [review]
PR 1000 - resultsets

awesome.  thanks for doing this!!
Attachment #8664976 - Flags: review?(cdawson) → review+
Attached file PR 1008 - global — 
PR for -global for review.
Attachment #8666126 - Flags: review?(cdawson)
Attachment #8666126 - Flags: review?(cdawson) → review+
Attachment #8664976 - Attachment description: PR 1000 → PR 1000 - resultsets
Attachment #8666126 - Attachment description: PR 1008 → PR 1008 - global
Ok, I think we are done for now, marking fixed per above 8 merges.

Many are already on stage/prod, the remaining ones will arrive there soon.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Everything has been deployed to stage and it looks fine there (and matches prod) so marking Verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: