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

[New Designs] Update Dashboard view to match new design #631

Merged
merged 18 commits into from
Dec 5, 2023

Conversation

aanorbel
Copy link
Member

@aanorbel aanorbel commented Oct 30, 2023

Fixes ooni/probe#2588

Proposed Changes

  • Update XML designs
  • Convert ProgressFragment to kotlin
. .
Screenshot_20231031_172528 Screenshot_20231031_172516
Screenshot_20231031_172436 Screenshot_20231031_172418

@aanorbel aanorbel changed the base branch from master to fix/dashboard-glitch October 30, 2023 20:08
@aanorbel aanorbel marked this pull request as ready for review October 31, 2023 10:26
@aanorbel aanorbel marked this pull request as draft October 31, 2023 10:27
@aanorbel
Copy link
Member Author

@agrabeli , We have been showing test_name on the test progress fragment but the new designs say we use the test suite name in the copy. I think we need a new copy for this section which may be different from the designs. What do you think.

@agrabeli
Copy link
Member

agrabeli commented Nov 1, 2023

Thanks @aanorbel for your question (though I feel like I may be missing some context). Is the goal to display (for example) Websites running in the test progress section, as opposed to Web Connectivity running? If that is the case, what new copy would be needed? Can we use the existing test suite copy (Websites, Instant Messaging, etc), or would we need new strings for Websites running, Instant Messaging running, etc?

Overall, why is this change needed? While I can see that perhaps having "obscure" experiment names is not preferable from a UX perspective, at the same time it may be more interesting to see (for example) that "the WhatsApp test is running, the Signal test is running..." than "Instant Messaging running"...

@aanorbel
Copy link
Member Author

aanorbel commented Nov 1, 2023

The new design calls for the test suite to be used instead of the test name. Wanted to know which side you're on.

@agrabeli
Copy link
Member

agrabeli commented Nov 1, 2023

@aanorbel I'd personally prefer keeping the test names in the progress bar as I personally like seeing which experiment is running, and which experiment doesn't run (for example, due to network issues etc). I also find it more informative seeing that (for example) "WhatsApp is running" than "Instant Messaging is running" (though I realize that not everyone is familiar with the test names, and that some test names are not very descriptive). But if the consensus is to replace the test names with the test suites, we could do that.

@aanorbel aanorbel marked this pull request as ready for review November 24, 2023 13:49
@bassosimone
Copy link
Contributor

bassosimone commented Dec 1, 2023

I second what @agrabeli write. In case of bugs, it helps us to know what was failing. "The app crashed while running WhatsApp" is more actionable than "the app crashed whole it was running Instant Messaging".

I think the main reason why this issue popped up is because the "Websites" group says "Web Connectivity". I would not be opposed to replace "Web Connectivity" with "Websites" because there's a 1:1 mapping.

On further reflection, for IM it definitely makes sense to keep the test names explicit because people know about that.

app/build.gradle Outdated Show resolved Hide resolved
Copy link
Contributor

@bassosimone bassosimone left a comment

Choose a reason for hiding this comment

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

Just minor changes

## Release Note
- Measurement engine synced with OONI Probe CLI
[v3.19.1](https://github.com/ooni/probe-cli/releases/tag/v3.19.1)
- Bug fixes and Improvements
## Proposed Changes

  - Replace `HeterogeneousRecyclerAdapter` with `RecyclerView.Adapter`
  - Add a `ViewModel` for state management
Base automatically changed from fix/dashboard-glitch to master December 5, 2023 14:53
Copy link
Contributor

@bassosimone bassosimone left a comment

Choose a reason for hiding this comment

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

🐳

@aanorbel aanorbel changed the base branch from master to dev/design-update December 5, 2023 17:08
@aanorbel aanorbel merged commit 5aac551 into dev/design-update Dec 5, 2023
3 of 5 checks passed
@aanorbel aanorbel deleted the issues/2588 branch December 5, 2023 17:14
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.

[New Designs] Update Dashboard view to match new design
3 participants