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

Input hides when annotation is correct for mnist example #782

Merged
merged 4 commits into from
May 31, 2022

Conversation

Etesam913
Copy link
Contributor

@Etesam913 Etesam913 commented May 27, 2022

Summary

In the mnist example clicking on the "Annotation Correct?" checkbox does not make it immediately clear to the user that they should type in the corrected annotation in the text input below.

This is fixed by only showing the "Corrected Annotation" text input when the "Annotation Correct?" checkbox is not ticked. When it is ticked, the text input is hidden.

Testing

task-1-vid.mov

From the video it should be easy to see that when the "Annotation Correct?" checkbox is ticked, the text input goes away.

@Etesam913 Etesam913 requested a review from pringshia May 27, 2022 21:22
@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 May 27, 2022
@Etesam913 Etesam913 changed the title ✨ Input hides when annotation is correct for mnist example Input hides when annotation is correct for mnist example May 27, 2022
@Etesam913
Copy link
Contributor Author

Am I not supposed to commit the package-lock.json because for some reason that seems to be adding many additions. I'm not sure why it's changing so much?

@pringshia
Copy link
Contributor

pringshia commented May 31, 2022

Am I not supposed to commit the package-lock.json because for some reason that seems to be adding many additions. I'm not sure why it's changing so much?

The changes seem pretty uneventful, mostly just dropping the dev: true property, which leads me to believe it's an npm version compatibility issue. Seems like others are also having this issue, but I see no defined solution anywhere.

The easy fix here, since you're not actually installing any new packages, is to just git revert the package-lock.json and remove it from this PR.

For the longer term though, what npm version are you using? For me it's:

$ npm --version
8.5.0

@Etesam913
Copy link
Contributor Author

I'll do git revert. I am using npm version 8.9.0.

@Etesam913
Copy link
Contributor Author

One thing I notice is that the package-lock.json seems to change when running python run_task.py

@pringshia
Copy link
Contributor

pringshia commented May 31, 2022

Looks good to me! Thanks for the video in the PR description.

I don't have a strong opinion on this either way, but another implementation we could add here is to reset the input box on toggling "Correct Annotation". However I'm not really sure if this UX change is worth the additional code.

A more sensible/obvious enhancement though would be to reset the input box when a new number is drawn. In that case, having a transient value from a previous drawing makes little sense.


That said, feel free to merge this in!

@Etesam913 Etesam913 merged commit b268cdb into main May 31, 2022
@JackUrb JackUrb deleted the clear-out-input-box-for-nmist branch July 13, 2022 18:39
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.

3 participants