Istanbul redesign branch

[tmcw]

Fixes #434 - this is a redesign of istanbul to simplify and beautify
reports.

• Demo

[tmcw]

cc @gotwarlost for a code review / design review

[ronkorving]

Very nice, I love it!

[gotwarlost]

I'm not the right person to review aesthetics :)

It certainly looks way more beautiful than the current set of the pages.
That said, I'll give you some feedback on the functional aspects of the new format

• the "code coverage by istanbul at date-time" line is too distracting at the top. People mostly don't need this information but it is useful sometimes. How big a deal is this to move it to the bottom and out of the way?
• IMO, the header takes up to much room. Partially due to the blurb on the top.
• Personally, I find things that are left-flush (instead of centered) to be more predictable for quick scans of pages. I do realize that that is not the "in" thing any more.
• As you can tell by the old report format, I'm also a big fan of column borders to delineate information in tables. I find the lack of borders makes the columns more difficult to grok

Thoughts?

[tmcw]

the "code coverage by istanbul at date-time" line is too distracting at the top. People mostly don't need this information but it is useful sometimes. How big a deal is this to move it to the bottom and out of the way?

Yep, moving this out of the footer was a pretty late decision: putting it back in the footer and making it much quieter sounds like a good idea.

Personally, I find things that are left-flush (instead of centered) to be more predictable for quick scans of pages. I do realize that that is not the "in" thing any more.

This design uses a max-width before it goes centered, so it will be left-flush on small displays. For larger screens, pushing everything hard to the left tends to look awkward.

As you can tell by the old report format, I'm also a big fan of column borders to delineate information in tables. I find the lack of borders makes the columns more difficult to grok

Yep, I'll do a second pass on the table styling.

[JuanCsfx]

Small comment, in the current HTML lcov-report, percentage values might have different number of decimals (e.g. 80% or 82.1% or 84.15%).

It would be great if as part of this redesign there could be a parameter to have all values using a specified number of decimals, in my opinion this makes it easier to compare numbers.

[tmcw]

I think that'd be out of scope for design - you would need to specify that parameter at generation time, right?

[JuanCsfx]

It is certainly borderline between generation and display.

There would be a way to dynamically change the values by inspecting all values belonging to certain classes (pct-high, pct-medium, etc) but this is probably out of scope if this redesign is restricted to changing CSS files

[MarkHerhold]

@tmcw Very nice work you are doing here. The whole community will greatly appreciate what you are doing. I remember the first time I used istanbul I wondered how many years old it was! 😆

I have some feedback if you are willing to hear it. I am by no means a designer or anything so take all of this with a grain of salt.

Color Scheme

I like the shade of green you chose. 👍 I really hate the purple on the code highlighting and in the table. Sorry. 😞 The color seems to obscure the code.

I understand that you are trying to make the theme where shades of purple mean untested, but I really just don't care for it. I actually like the old red color better.

Header

The new header looks so much better! I feel it is missing color for truly quick feedback though. I don't want to have to mentally translate 31.25% function coverage to "bad, needs work" every time I look at the percentages. I think coloring the percentage backgrounds would convey meaning a lot quicker.

For example, here is a quick mockup with palegreen, khaki, and lightcoral:

Footer

I don't see a screenshot of the footer. I assume it is gone, but if you do bring it back, please achor it the the bottom of the page. Footers in the middle of the page are silly!

Table

I think I would like the table better if it had a light border as @gotwarlost said.

Again, thanks for your work on this! 👍

[tmcw]

👍

I've updated the screenshots & live demo to reflect the latest style revision, that addresses much of the feedback:

• Reds are now more reddish and less bluish, and the red of syntax highlighting is lighter, so you can see the code better.
• The footer content is back on the bottom, making the header shorter. The footer sticks to the bottom of the page if the page content isn't >100% of the page height
• Tables now have column borders

I agree it'd be nice to have the header percentage numbers also be colored, but that would require a JavaScript-level change, and I'd like to keep this scoped tightly to CSS & HTML changes only.

[davglass]

I like the overall design changes, but I do miss the colors in the header tho. I really liked that the header color changed from red -> yellow -> green so a quick glance tells me that this page is ok. I can't get that from the current look.

[MarkHerhold]

Love the new screenshots with the new reds! 💯 Thanks for addressing the feedback @tmcw!

[tmcw]

@davglass the header still does that: it's a darker color concentrated in the bar:

[davglass]

@tmcw Ah - I didn't see that, the files I clicked on were all gray. Can you rebase to fix the conflicts? I'd like to pull this down, install it locally and run it on some of my larger code bases to see what I looks like with more code.

[tmcw]

@davglass 👍 just rebased

[jergason]

This looks so much better.

[gotwarlost]

I'm going to let @davglass be the czar for this PR. If he says it's ok, it is and vice versa :)

@tmcw you should reasonably expect that a small vocal minority might resist this change once it is merged so we need you at hand after the merge to handle any fallout and tweak as needed.

[mourner]

Love the improvements. The only thing I don't like is how metrics line up in the header — it makes them hard to read:

Rather than do a fixed 4-column layout, I would let the blocks flow and have fixed margin between each other. Closer to this (but with uniform margins, and the "ignored" block also fitting to the right if there's place left):

[mourner]

Also a minor thing, it would be nice for the line number field to be smaller so that the code lines up better with the header. The same thing for the files table — it would look nicer when lined up with the header.

[tmcw]

True, swapped the columns for inline, and made the table and numbers align

Aligning code to the header is not really doable or useful given the structure of the design - line numbers are right-aligned and the longest number will be at the bottom, so it's unlikely you'll see both the header and the aligned-line-number at the same time.

[mourner]

@tmcw at least you can remove fixed 50px column width so it looks like this:

Also, could you move "1 function ignored" to the rest of the blocks as well?

[tmcw]

Removed the 50px line width & moved the ignored message up

[mourner]

@tmcw looks perfect now.

[sterling]

Just my two cents here, but wouldn't it make sense for the overall coverage "progress bar" color to match the state it is in? Green has a particular meaning in the coverage report so it seems odd that they are always filled with green. For example, untested.js is in a "red" state (signified by the background color of the file name and the coverage bar). It makes more sense to me for the bar's filled color to match its state/background color (like a dark red rather than the current dark green).

[tmcw]

Green has a particular meaning in the coverage report so it seems odd that they are always filled with green.

As you can see in the screenshots and demo, the bar is green, grey, or red depending on context. Is there something else you're referring to? What you describe as the improvement is the current state, I'm pretty sure.

[tmcw]

Oh - the bar in the summary table, not the bar in the header.

[tmcw]

👍

medium:

low:

[sterling]

👍 perfect

[davglass]

I ran this locally on a larger internal project, two things stuck out when I took a look at the output.

First, the limiter makes it very hard to see when you have a deeper directory structure (4-5 levels deep). It looks much better without this limit and making the report fill the screen (with this it scrolled to the right). So removing the entire limiter class made it look much better.

.limiter {
  max-width: 960px;
  margin: 0px auto;
}

Second, the table listing of files without the row separator makes it very hard to tell what lines go where. It's hard to associate the numbers with the file without them. So adding something as simple as a bottom border like below makes it much more readable:

.coverage-summary td {
    border-right: 1px solid #999;
    border-bottom: 1px dashed #ccc;
}

Other than those two things, this is looking very good.

The app I tested it on was a fairly large internal one (just to show context on the feedback):

• 100% Statements (6088/6088)
• 99.9% Branches (3147/3150)
• 100% Functions (1151/1151)
• 100% Lines (6079/6079)

[sterling]

I tried out your branch on one of my own projects and this is how it looked in Chrome:

The sorting arrows are misaligned and it appears to be caused by long file names/paths.

[sterling]

Also, a suggestion:

I use a rather wide monitor and the 960px width feels a little limiting for tabular data--especially if the filenames/paths are long. Could it be a little more dynamic to utilize the space it is given on the window?

[kinday]

I dropped anything looks visually heavy to me.

White borders don’t stand out but still give a clue where the columns are. In this case we can add row separation and table remains neat. The last touch for borders is to remove ones separating related columns.

Also I removed charts’ borders and set chart height to 6 px. Charts are auxiliary; they aren’t the main content.

And what about 10 px horizontal padding? I removed it to look everything consistent.

[tmcw]

I'll look into removing the limiter class - it's pretty standard to have a limiter for large monitors (like GitHub's, for instance) since hugely-wide layouts are usually oddly spaced.

The sort-arrow breakage is a bug - I'll fix that.

Also I removed charts’ borders and set chart height to 6 px. Charts are auxiliary; they aren’t the main content.

I'm trying to redesign this as little as possible; each change from the status quo generally will come with some pushback, so my strategy is to make small high-value changes. You're welcome to do a second design push after this one, but I'd like to keep the scope down so that it can be merged in a reasonable timeframe.

[tmcw]

Updated the demo link & the branch, rebased against master and squashed to a single commit

• Ensures sorting arrows never drop to the next line
• Adds row borders
• Removes limiter

[davglass]

@tmcw This looks great now, I pulled and ran this on those larger internal projects again and I can now see the results much better now :)

I do believe that if/when we merge this it will have to be a major version bump. @gotwarlost are you opposed to making this a 1.0.0 or do you want to wait until your harmony branch is ready to make that leap? In the latter case then this would be a 0.4.0 release for sure.

[gotwarlost]

Strictly speaking these changes aren't backward incompatible unless someone is parsing HTML to get coverage output (in which case they deserve it!)

I'd go with 0.4.0 with props to @tmcw on the README as well as the contributions page. Major version changes without reason is an unnecessary overhead on all the modules that declare a 0.x dependency on istanbul. I want to reserve the major version change for some breaking instrumentation and source mapping changes that are coming soon (famous last words on the "soon" part :) )

[davglass]

@gotwarlost Sure, we'll do a 0.4.0 with this change.

@tmcw are you ready for this to be merged? I'll pull it locally, merge it with master and add the bits here and there for your contributions.

[tmcw]

👍 Yep, !