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 watch examples #2537

Merged
merged 3 commits into from
Nov 2, 2023
Merged

Fix watch examples #2537

merged 3 commits into from
Nov 2, 2023

Conversation

jay-es
Copy link
Contributor

@jay-es jay-es commented Oct 17, 2023

Description of Problem

The example code for watch may cause race conditions.
However, adding cleanup process makes the code complex and difficult to understand.
So I added a note.


Disable the input field until the request is resolved.

Proposed Solution

Additional Information

@netlify
Copy link

netlify bot commented Oct 17, 2023

Deploy Preview for vuejs ready!

Name Link
🔨 Latest commit dc6d449
🔍 Latest deploy log https://app.netlify.com/sites/vuejs/deploys/6543641d0e7e520008ff26d4
😎 Deploy Preview https://deploy-preview-2537--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.

@NataliaTepluhina
Copy link
Member

@jay-es maybe we could disable the input field until the request is resolved? This way we would be able to skip the cleanup process and also wouldn't require the info badge?

@jay-es
Copy link
Contributor Author

jay-es commented Oct 31, 2023

@NataliaTepluhina Thank you for your advice. I will try it!

@jay-es jay-es changed the title Add note to watch examples Fix watch examples Nov 1, 2023
@jay-es
Copy link
Contributor Author

jay-es commented Nov 1, 2023

@NataliaTepluhina I fixed. Could you please review it again when you have a moment?:pray:

@NataliaTepluhina
Copy link
Member

@jay-es thank you for adding the disabled state! I've updated your code a little to use the reactive property for disabled state instead of direct DOM manipulation through a ref. Also, let's not care about focusing a field here since at this point we haven't explained template refs to users yet. Could you please check the changes and let me know if you're okay with them? If yes, I'll proceed to merge

@jay-es
Copy link
Contributor Author

jay-es commented Nov 2, 2023

@NataliaTepluhina Much smarter code!
Thank you for fixing and please merge.

@NataliaTepluhina NataliaTepluhina merged commit c979157 into vuejs:main Nov 2, 2023
4 checks passed
@jay-es jay-es deleted the watch branch November 2, 2023 12:17
mostafa-nematpour referenced this pull request in vuejs-translations/docs-fa Nov 5, 2023
* Revert "Add Vue Toronto banner - schedule 2 (#2536)" (#2553)

This reverts commit 71ecfd5.

* Remove the app constant name from the method name (#2523)

* Remove the app variable name from the method name

To be consitent with the rest of the guide, it should be displayed the method name (with the dot accessor)
i.e.: https://vuejs.org/guide/essentials/application.html#mounting-the-app

* remove app const name from method

* Update rules-recommended.md (#2540)

docs: add missing div to seperate bad and good examples

* Update events.md - Diff between Options API and Composition API text (#2550)

* Update events.md - Diff between Options API and Composition API text

The emits support for object sintax for Options API text was displayed for both, Options and Composition API.

* Update events.md - in span instead of repeat code in different divs

* Update events.md - fix options-api class

* Update src/guide/components/events.md

Co-authored-by: Natalia Tepluhina <[email protected]>

---------

Co-authored-by: Natalia Tepluhina <[email protected]>

* Add missing ':' on duration property (#2556)

* docs: consistent colons in input labels (#2557)

* docs: consistent colons in input labels

* Update colons everywhere

* docs: use useByRole in example and fix typos (#2559)

getByRole validates the role of an element, giving us an extra free assertion

https://testing-library.com/docs/queries/about/#priority

* chore(deps): bump @vue/theme from 2.2.4 to 2.2.5 (#2555)

Bumps [@vue/theme](https://github.com/vuejs/theme) from 2.2.4 to 2.2.5.
- [Commits](https://github.com/vuejs/theme/commits/v2.2.5)

---
updated-dependencies:
- dependency-name: "@vue/theme"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix `watch` examples (#2537)

* Add note to `watch` examples

* fix watch examples

* fix: fixed disabling an input

---------

Co-authored-by: NataliaTepluhina <[email protected]>

* imported and changed twitter icon to new logo (#2548)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Evan You <[email protected]>
Co-authored-by: Leo <[email protected]>
Co-authored-by: Jérémie LITZLER <[email protected]>
Co-authored-by: Natalia Tepluhina <[email protected]>
Co-authored-by: cabbageshop <[email protected]>
Co-authored-by: Andrew <[email protected]>
Co-authored-by: Yakir Gottesman <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jun Shindo <[email protected]>
Co-authored-by: stevenks17 <[email protected]>
nazarepiedady pushed a commit to nazarepiedady/vue3-docs that referenced this pull request Nov 30, 2023
* Add note to `watch` examples

* fix watch examples

* fix: fixed disabling an input

---------

Co-authored-by: NataliaTepluhina <[email protected]>
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