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

Add terse/verbose toggle to results and interop (dir-level) views #486

Closed
wants to merge 9 commits into from

Conversation

mdittmer
Copy link
Contributor

Fixes #422.

@wpt-pr-bot
Copy link

Staging instance deployed by Travis CI!
Running at https://dir-verbose-dot-wptdashboard-staging.appspot.com

@lukebjerring lukebjerring requested a review from foolip August 28, 2018 17:11
@lukebjerring
Copy link
Contributor

So, without diving into the code just yet:
I'm not sure we should have the toggle on directories. While some paths are (too) long, the ellipsis should rarely cause ambiguity at the folder level, and hovering resolves that in any case.

@Hexcles
Copy link
Member

Hexcles commented Aug 28, 2018

I noticed a small UI quirk: expand the results in https://dir-verbose-dot-wptdashboard-staging.appspot.com/results/credential-management , and you'll see the first lines of the long test names have a small indent that doesn't exist on staging.wpt.fyi.

@Hexcles
Copy link
Member

Hexcles commented Aug 28, 2018

A more general request: if we are changing the table layout, we should take the chance to improve the readability of the UI, namely to separate the rows more clearly in the expanded view.

@mdittmer
Copy link
Contributor Author

mdittmer commented Sep 6, 2018

I'm not sure we should have the toggle on directories. While some paths are (too) long, the ellipsis should rarely cause ambiguity at the folder level, and hovering resolves that in any case.

@foolip I think you requested this feature. WDYT?

@foolip
Copy link
Member

foolip commented Sep 6, 2018

@foolip I think you requested this feature. WDYT?

https://dir-verbose-dot-wptdashboard-staging.appspot.com/results/fullscreen/api?complete=true is a case where this feature helps a bit, but now I see that at the top level the feature is a bit confusing. It just slightly changes the layout and it might appear to just not do anything useful.

So, I'm going to apologize (sorry!) for not quite thinking this through, and agree with @lukebjerring that probably we don't need this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants