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

UX improvements for "Don't ask again" #687

Merged
merged 26 commits into from
Mar 11, 2024
Merged

UX improvements for "Don't ask again" #687

merged 26 commits into from
Mar 11, 2024

Conversation

petersalomonsen
Copy link
Collaborator

@petersalomonsen petersalomonsen commented Mar 5, 2024

Preview environment

https://near.org/devgovgigs.petersalomonsen.near/widget/app?page=post&id=3135

Implementation plan

Tests

  • upgrade to latest near-social-vm (near-bos-webcomponent) in tests
  • Set up test scenarios with "Don't ask again" configured ( accesskey to devhub.near in localstorage, and widget permission in indexeddb cache ( VM local storage) )
  • Handle that the VM is not able to detect the widget when bos-loader is used, by modifying RPC responses from social.near / get
  • Modify response for view_access_key_list RPC call to avoid wallet redirect
  • Modify response for sending transaction, so that the test get a response that the transaction is complete
  • Wait for contents to refresh after cache invalidation, and assert that there is an indicator while waiting

Implementation

  • Hide submit/cancel buttons when submitting the transaction
  • Ensure that these buttons should not be removed if there is a confirmation dialog ("Don't ask again" is not enabled), and the user selects cancel. BLOCKED Depends on Callback for Near.call NearSocial/VM#105
  • Indicator that automatic refresh is still going on after the transaction is completed, to avoid that the user thinks the page needs to be reloaded
  • Clicking "Like" should also prevent double clicking, and also indicate something while refreshing is still going on

Screen videos ( generated by test runner )

The following video shows the hiding of the submit and cancel button when clicking the submit button, and it also shows a loading indicator all the way until the contents of the page is submitted. Note that it might seem confusing that you don't see the submitted comment after refresh, but this is only the behaviour in the test-runner, where there is no actual transaction submitted.

dontaskagaincommentfast.mp4

In the next video everything is quite slow, just like in the video from the issue where I pointed out the problem. This fix does not intend to solve the slowness problem that happens when interacting with a long thread like this one, but rather makes sure that there are indicators showing that there are things going on.

dontaskagainpostfix.mp4

This fix will also introduce a new problem. If "Don't ask again" is not enabled, and you close the transaction confirmation dialog ( NearSocial/VM#105 ), then the loading indicator is still there, and the submit button is still hidden. I haven't found any way to solve this as long as we don't have a callback method for when the transaction confirmation dialog is closed.

Here's a video showing that problem:

dontaskagaincommentcloseconfirmation.mp4

Finally here's a video demonstrating how clicking the like button is handled. Here you can see that double-clicks are prevented by hiding the button immediately when it's clicked. Also you can see that the loading indicator is showing until the new like is updated.

dontaskagainlike.mp4

fixes #680

@petersalomonsen
Copy link
Collaborator Author

petersalomonsen commented Mar 7, 2024

A challenge I discovered when working on this is that setting flags at https://near.org/flags to http://127.0.0.1:3030 where there is a bos-loader running prevents the transaction confirmation dialog from detecting the widget, which is needed by the "Don't ask again" feature (it needs to store which widget that has permission to skip transaction confirmation).

It looks like in the screenshot below, and notice that it says "Don't ask again for sending similar transactions by", but no widget name.

image

Because of this the only way to verify the fix is currently by deploying to a preview environment, or by intercepting the RPC calls to retrieve the widget in the Playwright test runner. I'm currently looking into mocking the RPC call to return the widget the local development environment.

@petersalomonsen
Copy link
Collaborator Author

The latest commit is modifying responses from RPC calls to social.near / get and is replacing with the widget in the local development environment. It works!

@petersalomonsen
Copy link
Collaborator Author

Here's a screen video showing the first part of fixing this, which is removing the submit and cancel buttons from the view when submitting the transaction. Also note the don't ask again toast in the bottom right.

dontaskagainremovebuttons.mp4

The next step is to ensure that these buttons should not be removed if there is a confirmation dialog, and the user selects cancel. I will add this to the acceptance criteria.

@petersalomonsen petersalomonsen requested a review from frol March 9, 2024 21:43
@petersalomonsen petersalomonsen marked this pull request as ready for review March 9, 2024 22:21
@frol
Copy link
Collaborator

frol commented Mar 10, 2024

@petersalomonsen Would it be easy to disable the buttons and add loader inside the button instead of completely removing the button? This is what I mean:

image

Regarding the blocker - the only solution I can think of is to have 15 seconds timeout for this loader state and also have an additional check if the post is already recorded on chain - this complicates the logic quite a bit, so I would only implement the timeout part if possible.

@petersalomonsen
Copy link
Collaborator Author

@frol Having the loading spinner inside the button, and disabling it, was an easy change. The preview environment is updated with this. I did set the spinner on the left side of the button as it is in the Bootstrap docs ( https://getbootstrap.com/docs/4.2/components/spinners/ ).

I haven't implemented the timeout yet. It feels rather hacky. I believe your suggestion could work, though for long threads even 15 seconds might not be enough, which you also can see in the video with the long thread. I'm not sure how critical this is, but I feel the appropriate would be to start implementing support for a callback in the VM when the confirmation dialog is closed.

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.

Indicator while waiting for post to be submitted and page refreshed ( when "Don't ask again" is enabled )
2 participants