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 feedback component to loading page #888

Closed
wants to merge 3 commits into from

Conversation

Etesam913
Copy link
Contributor

@Etesam913 Etesam913 commented Aug 9, 2022

Overview

A very common circumstance where feedback might want to be received is during loading.

There are times where a task my be stuck in loading forever. The researcher should be able to know directly if a large chunk of workers are not able to see the task.

This was brought up to me by @EricMichaelSmith

Changes:

  • Feedback component shows after loading for 5 seconds (should this number be adjusted? not exactly sure about the average loading times).

Video

feedback-on-loading.mp4

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 9, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 9, 2022

Codecov Report

Merging #888 (abb44a8) into main (b165bb2) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #888      +/-   ##
==========================================
- Coverage   64.65%   64.60%   -0.06%     
==========================================
  Files         108      108              
  Lines        9312     9312              
==========================================
- Hits         6021     6016       -5     
- Misses       3291     3296       +5     
Impacted Files Coverage Δ
mephisto/abstractions/architects/mock_architect.py 88.23% <0.00%> (-2.62%) ⬇️
mephisto/data_model/unit.py 77.59% <0.00%> (-0.55%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Etesam913
Copy link
Contributor Author

@EricMichaelSmith Is this kind of what you expected?

@JackUrb
Copy link
Contributor

JackUrb commented Aug 9, 2022

This is cool! Only issue is that the most common reason this happens is because the mephisto router is alive, but the Mephisto server that was associated with it no longer is connected. After #829 however then this would cover the actual loading use cases.

@EricMichaelSmith
Copy link
Contributor

@EricMichaelSmith Is this kind of what you expected?

Yeah! This is really cool to see, and it'll let us get feedback about broken HITs. I feel like, in my experience, emails about broken HITs are at least as common as all other email topics put together...

@pringshia
Copy link
Contributor

pringshia commented Aug 9, 2022

This is a clever use case for the new Feedback component!

I would suggest a 10 second delay, 5 seems a little too quick to me. It would also be nice to have the timeout value as a constant, and perhaps we can expose that at the top of the app.jsx file. (in the future other configurable constants could also be colocated there)

✏️ Fixed typo 'propertly' -> 'properly'

⏳ Increased feedback box timeout from 5000ms -> 10000ms
@facebook-github-bot
Copy link
Contributor

Hi @Etesam913!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@Etesam913 Etesam913 closed this Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants