Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[test-case-reporting] Make test run list page prettier with CSS #965

Merged
merged 16 commits into from
Aug 10, 2020

Conversation

Rafer45
Copy link
Contributor

@Rafer45 Rafer45 commented Aug 5, 2020

Works toward #964.
Looks like this:
image

@Rafer45 Rafer45 requested a review from rcebulko August 6, 2020 19:32
@rcebulko
Copy link
Contributor

rcebulko commented Aug 6, 2020

I think one thing that would work well visually is if each row started with a small tile indicating the status, like a green/red/yellow/orange icons or something similar (or even just a yellow box with the word 'SKIP'). Something like this might work/be a good starting point:

.status-badge {
  width: 60px;
  padding: 2.5px;
  display: inline-block;
  font-family: sans-serif;
  font-variant: small-caps;
  color: white;
}
.status-pass { background-color: green; }
.status-fail { background-color: orange; }
.status-skip { background-color: gray; }
.status-error { background-color: red; }
<span class='status-badge status-pass'>Pass</span>
<span><a href=''>this is a test case blah blah</a></span>

Hopefully this gets close to working out of the box, have not tested yet

@Rafer45
Copy link
Contributor Author

Rafer45 commented Aug 7, 2020

Hopefully this gets close to working out of the box, have not tested yet

It mostly worked, but the badge looked a little weird, so I tried applying the color to the background of the collapsible -- let me know what you think.

@rcebulko
Copy link
Contributor

rcebulko commented Aug 7, 2020

Hopefully this gets close to working out of the box, have not tested yet

It mostly worked, but the badge looked a little weird, so I tried applying the color to the background of the collapsible -- let me know what you think.

Screenshot? :)

@Rafer45
Copy link
Contributor Author

Rafer45 commented Aug 7, 2020

Hopefully this gets close to working out of the box, have not tested yet

It mostly worked, but the badge looked a little weird, so I tried applying the color to the background of the collapsible -- let me know what you think.

Screenshot? :)

Of what looked weird, or to the collapsibles with background colors? I have screenshots of the latter at the top.

@rcebulko
Copy link
Contributor

rcebulko commented Aug 7, 2020

The reason I prefer the badges over just coloring the collapsibles is a) the color-to-status relation is non-obvious unless you provide a legend or something and b) color-only approach is very inaccessible to color-impaired users and screen-readers

@Rafer45
Copy link
Contributor Author

Rafer45 commented Aug 7, 2020

Makes sense, let's go with the badge approach. It currently looks like this --
image

@rcebulko
Copy link
Contributor

rcebulko commented Aug 7, 2020

Try adding the following rules to .status-badge

margin-right: 10px;
text-align: center;
font-weight: bold;

And try to put it inside the collabsible label section so it lines up with the test name, then we can remove the "had status X" at the end

@Rafer45
Copy link
Contributor Author

Rafer45 commented Aug 7, 2020

@rcebulko, would you support swapping the colors for fail and error, so that error is orange and fail is yellow?

@Rafer45 Rafer45 requested a review from rcebulko August 7, 2020 18:51
@rcebulko
Copy link
Contributor

rcebulko commented Aug 7, 2020

@rcebulko, would you support swapping the colors for fail and error, so that error is orange and fail is yellow?

However you have it in the screenshot in the description right now looks good to me 👍

@Rafer45
Copy link
Contributor Author

Rafer45 commented Aug 7, 2020

Ok, keeping the colors like that, then.

@rcebulko
Copy link
Contributor

rcebulko commented Aug 7, 2020

I like the collapsibles too. Right now it's pretty empty, but down the line I can see including test case stats, metadata about the build/PR, duration, and even spaces where people could leave comments with related issues, error info, or reasons why something was skipped. Seems super flexible

@Rafer45 Rafer45 requested a review from rcebulko August 10, 2020 16:30
@Rafer45 Rafer45 merged commit 3adf768 into ampproject:master Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants