Corrected summary formatting problems

[AlexArchive]

@mwhelan asked me to have a look at this. I managed to correct the alignment problems across modern web browsers.

This is what the summary looked like as of commit fe1217

This is what the summary looks like now

According to @mwhelan the approval tests are in a state of disarray right now. I think I need to update the *approved.txt files to include the updated (minified) css. What is the best (automated?) way of doing this?

[mwhelan]

Well, I don't know if I would quite say they were in disarray but yeah, they are failing when you download the source from github. :-)

Great work @byteblast ! Huge improvement and thanks for investigating and fixing the issue. It looks so much better! Just to be really pedantic, I wonder if it is possible for the numbers to be right aligned?

[AlexArchive]

Sorry. I used the wrong word 🐑.

Hm. I do not know how to achieve that in this context off-hand. I mean, I can get it to work

.summaryCount {
    position: absolute;
    width: 30px;
    text-align: right;
}

but it involves setting a fixed width. Settings a fixed width disturbs the original layout a little bit and will cause breakage when the number needs more space than I defined. I would need to do some more research. Shall we revisit this at a future date if you still deem it important?

[mwhelan]

I think right alignment is quite important for numbers and the new screenshot looks a lot better. If it can support 4 digits then it should be fine?

[AlexArchive]

I think it is quite reasonable to accommodate 4 digits only. Hold on. (Do note as well that there could be a better way of doing this that I do not yet know about.)

In the mean time could you please answer my earlier question?

I think I need to update the *approved.txt files to include the updated (minified) css. What is the best (automated?) way of doing this?

Must I update the *approved.txt files?

[mwhelan]

No, in fact I don't think you should as the file location seems to be dynamic depending on your local file location. I just meant that you can do that if you want to get the tests passing, which you would if you are making changes to the report and using those tests to verify the changes. I think @JakeGinnivan and @MehdiK will have more idea than me about the cause of those tests failing and the best solution, so don't worry about it yourself.

[mwhelan]

Sorry, I've only partially answered your question, but the same applies. Wait for @MehdiK or @JakeGinnivan to weigh in, as I'm not too sure about approval tests.

[AlexArchive]

I just meant that you can do that if you want to get the tests passing

I understood that much 😕. The *approved.txt files contain the old (and broken) CSS. I think the CSS in those files might need to be updated is all. I am new to this code base so I don't really know.

Anyhoo, this is what the summary looks like now

If you exceed 4 digits then the formatting will break, like I said it would

What do you think?

[AlexArchive]

Many thanks for your guidance by the way.

[mwhelan]

Yes, I think you are probably right that the CSS needs to be modified in the approved file. So, you should go ahead and change that in the approved file. (I would say to approve the received file but the file location won't match the build server so in this case probably just change the CSS in the approved file).

I really like what you've done and I think we should go ahead with the change. I think it's very unlikely we would need more than four digits, but if the unlikely does happen, it actually still looks pretty good with the broken formatting.

[AlexArchive]

Done.

[mwhelan]

Thanks @byteblast for a valuable contribution and for persisting with getting it just right.

[AlexArchive]

No problem mate.