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

Bug/finishingtheform 502 #539

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

shivpratikhande
Copy link

@shivpratikhande shivpratikhande commented Oct 1, 2024

Fixes

Description

This pull request addresses the issue with the "Done" button in the chooser form, where the screen shakes without providing a clear indication of what is missing. This PR aims to improve user experience by providing clear feedback after clicking "Done."

Technical details

  • Updated the logic for the "Done" button to prevent the shake effect if the form is correctly filled out.
  • Added a pop-up message to indicate successful completion.
  • Adjusted the visual feedback by graying out the left panel and modifying the button text.

Tests

  1. Fill out the chooser form completely.
  2. Click the "Done" button.
  3. Verify that a success message appears in the right panel.
  4. Confirm that the left panel is grayed out and only the "Start again" button remains active.

Screenshots

Before
Screenshot from 2024-10-01 19-37-29
After
Screenshot from 2024-10-01 19-37-59

Demo Video

Here’s a demo of the functionality:

done.mp4

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the default branch of the repository (main or master).
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added or updated tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

For the purposes of this DCO, "license" is equivalent to "license or public domain dedication," and "open source license" is equivalent to "open content license or public domain dedication."

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@shivpratikhande shivpratikhande requested review from a team as code owners October 1, 2024 14:10
@shivpratikhande shivpratikhande requested review from TimidRobot, possumbilities and zackkrida and removed request for a team October 1, 2024 14:10
Copy link

netlify bot commented Oct 1, 2024

Deploy Preview for creativecommons-chooser ready!

Name Link
🔨 Latest commit 734e44d
🔍 Latest deploy log https://app.netlify.com/sites/creativecommons-chooser/deploys/66fc02bbaa2901000839691e
😎 Deploy Preview https://deploy-preview-539--creativecommons-chooser.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@possumbilities
Copy link
Contributor

HI @shivpratikhande thank you for this contribution! I'm not sure adding a popup here is great UX.

Also, one of the key issues in the original PR is that shaking an element is often utilized to communicate there is something missing in that element. In the case of the Chooser, its current behavior of shaking is to draw attention to the area that it is "ready". This creates the outlined confused in that Issue, because that's misaligned expectations of behavior.

This PR leaves the shaking behavior in and doesn't address the confusion that behavior causes. I'd suggest rethinking the approach a bit here.

If you want to talk through it further it might be helpful to talk through your approach on the Issue before moving forward with more PR work?

@possumbilities possumbilities removed request for a team, TimidRobot and zackkrida October 1, 2024 15:02
@shivpratikhande
Copy link
Author

After clicking the button, the form area greys out and becomes unresponsive, except for the 'Restart Again' button. This indirectly draws attention to the shaking element, suggesting that the action is complete, which was the intended behavior or a bug in the previous code, and also this was one of the requirement.

@possumbilities
Copy link
Contributor

@shivpratikhande from a UX perspective the grey-ing out doesn't fully resolve the confusion caused by the shaking. Please reconsider the approach here.

@shivpratikhande
Copy link
Author

What if I align this popup message to the right, just below the copyright license?

@possumbilities
Copy link
Contributor

@shivpratikhande There's a lot of back-and-forth on approach here that I think is best captured on the original Issue. It might be best to propose alternate routes of solution there.

There are complex issues at play here, where conflicting behaviors are fixing one thing, but adversely affecting another and a solution here would need to resolve the original Issue, while also adjusting for any collateral damage to the UX.

The likely culprit is that the UX of the Chooser at "completion state" in broken across several dimensions, namely it shouldn't need to shake to draw user attention, but its current design requires that in some way. However, shaking an element has become synonymous conventionally with showing there is an error in that element, but in this case there isn't an error in that element, but in its sibling which isn't shaking. Adding pop-ups and greying out the inputs doesn't resolve the root issues here, it just adds more layers of trying to work from a foundation of UX that may not be fully stable.

My suggestion here might be to take a step back, and work up a route of solution and discuss it on the Issue and then come back to a PR or modify this PR further.

It is also possible that the original Issue needs to be moved to status: blocked in that the interwoven complexity of change necessary to resolve that specific Issue is much larger than anticipated and other UX or layout work needs to be addressed prior.

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

Successfully merging this pull request may close these issues.

[Bug] After finishing the form, clicking "Done" doesn't do anything
2 participants