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 UI for Psiphon #88

Merged
merged 21 commits into from
Jan 29, 2020
Merged

Add UI for Psiphon #88

merged 21 commits into from
Jan 29, 2020

Conversation

sarathms
Copy link
Contributor

@sarathms sarathms commented Dec 3, 2019

This adds UI for Psiphon test. As a dependency, this also has UI for the newly implemented Circumvention test group itself.

Fixes ooni/probe#957

@sarathms sarathms self-assigned this Dec 3, 2019
@hellais hellais added the priority/high Important issue that needs attention soon label Jan 6, 2020
@hellais
Copy link
Member

hellais commented Jan 17, 2020

Some conflicts need to be resolved with this PR before merging.

@sarathms sarathms requested a review from hellais January 28, 2020 16:42
Copy link
Member

@hellais hellais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at the code and left some comments.

I also think that we should re-order the cards such that the Circumvention tests are after the IM tests and before the Performance tests. Otherwise they are burried under a page scroll.

See:
Screenshot 2020-01-28 at 18 01 58

@sarathms
Copy link
Contributor Author

Another thing I forgot to flag was that we don't have the animation for Circumvention group in here. https://github.com/ooni/design-system/tree/master/components/animations

@hellais
Copy link
Member

hellais commented Jan 28, 2020

More feedback here:

the icon for the running circumvention test is not the correct one. See:
Screenshot 2020-01-28 at 18 05 02

I think this is because we don't have an animation for it yet, can you use in the meantime as a placeholder the static Circumvention icon?

@hellais
Copy link
Member

hellais commented Jan 28, 2020

When I click on the row of a result I get the following error:
Screenshot 2020-01-28 at 18 08 16

I suspect this is due to the fact that my ooniprobe is too recent and it's failing on the line I commented on above: #88 (comment)

@sarathms sarathms requested a review from hellais January 29, 2020 11:53
@sarathms
Copy link
Contributor Author

For this PR, hiding results for unsupported tests.

@sarathms
Copy link
Contributor Author

The 'test is running' animation will be replaced with group icons for groups without an animation. But, it looks like the icons don't scale very well.
Screenshot from 2020-01-29 08-32-49

@hellais
Copy link
Member

hellais commented Jan 29, 2020

I tested this and it's looking good.

We should do a bit more polish to it based on the feedback from @holantonela in here: ooni/probe#967

Copy link
Member

@hellais hellais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more polish of the UI should be done, but I say as an MVP we merge this.

@sarathms sarathms merged commit 51a1de9 into master Jan 29, 2020
@hellais hellais deleted the ux/circumvention-test-run branch March 6, 2020 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/high Important issue that needs attention soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write UI for the Psiphon test result in Desktop
2 participants