This repository has been archived by the owner on Feb 22, 2023. It is now read-only.
Use vue-portal for VTeleport #1716
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Storybook and Tailwind configuration previews: Ready Storybook: https://wordpress.github.io/openverse-frontend/_preview/1716 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
openverse-bot
added
🕹 aspect: interface
|
Size Change: +3.46 kB (0%) Total Size: 1.36 MB
ℹ️ View Unchanged
|
dhruvkb
approved these changes
Aug 22, 2022
There was a problem hiding this comment.
The SSR docs for Vue Portal suggest disabling it on the server and only running it on the client side using <no-ssr>.
I'm all for replacing a custom implementation with a tried and tested library (approving for that) but the docs seem to suggest that our issue with SSR wouldn't go away after the switch.
sarayourfriend
approved these changes
Aug 22, 2022
Comment on lines
-16
to
+17
| class="md:block md:truncate md:text-left md:ms-2" | ||
| :class="isHeaderScrolled ? 'hidden' : 'block truncate text-left ms-2'" | ||
| class="md:block md:truncate md:ms-2 md:text-start" | ||
| :class="isHeaderScrolled ? 'hidden' : 'block truncate ms-2 text-start'" |
There was a problem hiding this comment.
Good catch, surprised the change didn't require updating snapshot tests though 🤔
There was a problem hiding this comment.
I caught myself using left and right classes a couple of days ago 🙈
@dhruvkb, I believe this part of docs refers to the specific (older than Nuxt) implementation, the SSR for Vue. The library also has a Nuxt module, and it works pretty well. At least, I didn't notice errors. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Hopefully:
Fixes #1078 by @ sarayourfriend
Fixes #615 by @ obulat
Fixes #1417 by @ AetherUnbound
Description
This PR replaces the custom implementation of Teleport with
vue-portallibrary that hopefully handles the SSR better. The library hasn't been updated for some period of time, but it's probably caused by the fact that Vue 2 is not the default anymore, and Vue3 has a native implementation ofTeleport.Testing Instructions
Currently, if you look at the CI Playwright tests' log, you'll see a lot of warnings like these:
The CI logs for this PR shouldn't have warnings about deleting portal targets, and the tests should not fail due to crashes.
Checklist
Update index.md).main) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin