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

fix(tutorial): race condition on step-10 #2567

Merged

Conversation

ubugeeei
Copy link
Member

@ubugeeei ubugeeei commented Nov 13, 2023

Description of Problem

In the code for step 10 of the tutorial, there is an issue where rapidly clicking the button can result in the results not matching the id.

スクリーンショット 2023-11-13 23 44 36

Proposed Solution

There are several possible solutions.

  • Disable the button during loading
  • Hide the button while loading
  • Add a cleanup process.

As referred to in #2537, adding a cleanup process would require explaining more, which I'm concerned might deviate from the original purpose.
Therefore, I believe one of the first two options mentioned would be preferable.

In this PR, I tried fixing it in the direction of disabling the button, as initially suggested!

Behavior after the correction:

2023-11-13.23.56.50.mov

Additional Information

If you have any better suggestions for the fix, please feel free to propose them! 😄

Copy link

netlify bot commented Nov 13, 2023

Deploy Preview for vuejs ready!

Name Link
🔨 Latest commit 121a37e
🔍 Latest deploy log https://app.netlify.com/sites/vuejs/deploys/65523b67a63db5000830e7ad
😎 Deploy Preview https://deploy-preview-2567--vuejs.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.

Copy link
Member

@NataliaTepluhina NataliaTepluhina left a comment

Choose a reason for hiding this comment

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

@ubugeeei thank you! I think this is the cleanest way to handle the race condition here

@NataliaTepluhina NataliaTepluhina merged commit 8087e30 into vuejs:main Nov 14, 2023
4 checks passed
nazarepiedady pushed a commit to nazarepiedady/vue3-docs that referenced this pull request Nov 30, 2023
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.

2 participants