-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
🚀 Feature: Highlight browser failures in red #792
Comments
Hi. I am new here and would like to contribute. Thank you! :) |
Hi, is this available to work on? |
Sure |
created pr #4082 |
updated pr with new code and included screenshots. Please let me know if anything else needs to be changed |
I see a ref to the new pr but just wanted to make sure you guys were aware. created a new pr for this issue: |
Why isn't this merged? |
@outsideris Hi can I get an update on my PR? |
@dhuang612 We missed it. I will check your PR. |
Hi @outsideris is it available to work on? I don't see any progress from the latest PR above |
#4148 looks like it can be resurrected, I'll see what I can do tomorrow :) |
FYI #5218 is ready for review :) |
Thanks @mark-wiemer! Coming over from #5218 (comment): my personal take is that the best implementation for this issue would be one that doesn't change page layout. From that comment: the taller the summary section, the less fits on the page in smaller viewports. A lot of people are accustomed to the placement as-is. The border treatment you have in #5218 could get the job done, but colored borders tend to necessitate margin & padding to look good. Which means it might be hard to get that to look good without changing layout. Proposal: how about modifying the color on failure for text & progress circle stroke? Say,
From an accessibility perspective, the summary already indicates the pass/fail status with |
Text color changes sound fine by me! I'll see what I can do :) |
How do you feel about adding the X to indicate a failure as well? I thought that was a nice super clear item that stands out even in black and white, and it doesn't make the view any taller! |
I like that idea 🙂. Thinking: there's already a green ✔ or red X put on individual tests once their status is known. So this would be putting the same treatment on the summary?
Is that what you're thinking? 👍 from me on that if so. |
I think that would be best, yes. I'd add a green check even if some tests were skipped. Basically once all tests complete, green check unless any test failed. If any failures, red X :) |
Hey @JoshuaKGoldberg I'm not sure about coloring the progress circle and its inner text as red or green. To me that could be read as "all tests failed" or "100%" of tests failed, where someone may expect a "99%/1%" text to indicate 1% failed. Additionally, highlighting the whole circle red might be read as "all tests failed" when that's not really what it means. For now, I think the green text of passes and failures, plus the new result indicator, is sufficient: |
#5222 is ready for review :) |
Oh, good point. Agreement from me on not changing it for now then 🙂.
Looks great! What do you think about also coloring the numbers, like |
coloring the numbers as well works for me, let me update it :) |
to make it more obvious without wasting eye-movement effort to look at the count haha. I prefer github.com/visionmedia/mocha-matrix for this reason but it's not a good fit as a default
The text was updated successfully, but these errors were encountered: