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: Vue 2/3 lifecycle type errors and script tag cleanup #79

Merged
merged 3 commits into from
Jun 20, 2023

Conversation

ubercj
Copy link
Contributor

@ubercj ubercj commented Jun 5, 2023

Purpose of PR

This PR is to address a couple of bug issues:

#73

  • googleTranslateSelectObserve and htmlAttrLangObserve are now checked for null during onBeforeUnmount. It looks like this is actually already done in the React package, so the Vue packages should be the same now.

#78

  • The script tag from createScript is now removed in onBeforeUnmount. Then, when the page changes, a new script tag is inserted during onMounted and the script is run to reinject the .goog-te-combo element in the DOM. This ensures that the hidden select tag (.goog-te-combo) is present in between page loads in a SPA, for example.

PR Checklist

  • I have read the relevant readme.md file(s)
  • Tests are added/updated/not required
  • Tests are passing
  • Storybook stories are added/updated/not required
  • Usage notes are added/updated/not required
  • Doesn't contain any sensitive information

@changeset-bot
Copy link

changeset-bot bot commented Jun 5, 2023

⚠️ No Changeset found

Latest commit: 0b96387

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jun 5, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
google-translate-select-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 5, 2023 3:23pm

@TooColline
Copy link

Nice work @ubercj. Let's get this merged in @i7eo if it looks good to you

@ubercj
Copy link
Contributor Author

ubercj commented Jun 19, 2023

@i7eo have you had a chance to look at this?

Let me know if any edits are needed to get this merged in. It would be great to get this fixed so I can start using this package on my site

@i7eo
Copy link
Owner

i7eo commented Jun 20, 2023

@i7eo have you had a chance to look at this?

Let me know if any edits are needed to get this merged in. It would be great to get this fixed so I can start using this package on my site

Sorry guys, I've been very busy lately. I checked your pr and there is no problem, it has been merged

@i7eo i7eo merged commit ee301a7 into i7eo:master Jun 20, 2023
@i7eo
Copy link
Owner

i7eo commented Jun 20, 2023

You can check version 0.1.2, Thanks @ubercj 👏

@ubercj
Copy link
Contributor Author

ubercj commented Jun 20, 2023

Sweet, thanks @i7eo! I updated to v0.1.2 and it's working great for me 👍

@TooColline
Copy link

Has the change been effected for the Vue 2 counterpart? i.e. @google-translate-select/vue2 forgot to review that part because I'm still experiencing it on the Vue 2 package

cc: @ubercj @i7eo

@i7eo
Copy link
Owner

i7eo commented Jun 21, 2023

Has the change been effected for the Vue 2 counterpart? i.e. @google-translate-select/vue2 forgot to review that part because I'm still experiencing it on the Vue 2 package

cc: @ubercj @i7eo

Yeah, you can bump your @google-translate-select/vue2 version from 0.1.1 to 0.1.2.

@i7eo
Copy link
Owner

i7eo commented Feb 20, 2024

@all-contributors please add @ubercj for code

Copy link
Contributor

@i7eo

I've put up a pull request to add @ubercj! 🎉

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.

3 participants