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

Replace nuxtjs/svg with nuxtjs/svg-sprite #2135

Merged
merged 5 commits into from May 29, 2023
Merged

Conversation

obulat
Copy link
Contributor

@obulat obulat commented May 19, 2023

Fixes

Fixes #1849 by @obulat

Description

This PR replaces "nuxtjs/svg" library that does not have support for Nuxt 3 with "nuxtjs/svg-sprite". We already use it for icons.
To use it for the images of various sizes, we have to add a viewBox to the VSvg components - I couldn't find a better solution, unfortunately.

Testing Instructions

All of the Visual Regression tests in the CI should still pass because there should be no visible change.
So, to test this PR you would need to just read through the code and check that the CI passes.

Checklist

  • My pull request has a descriptive title (not a vague title likeUpdate index.md).
  • My pull request targets the default branch of the repository (main) or a parent feature branch.
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added or updated tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no visible errors.
  • I ran the DAG documentation generator (if applicable).

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@openverse-bot openverse-bot added this to In progress in Openverse PRs May 19, 2023
@github-actions github-actions bot added 🧱 stack: documentation Related to Sphinx documentation 🧱 stack: frontend Related to the Nuxt frontend labels May 19, 2023
@github-actions
Copy link

github-actions bot commented May 19, 2023

Size Change: -9.43 kB (-1%)

Total Size: 839 kB

Filename Size Change
./frontend/.nuxt/dist/client/app.js 123 kB -4.82 kB (-4%)
./frontend/.nuxt/dist/client/app.modern.js 115 kB -4.89 kB (-4%)
./frontend/.nuxt/dist/client/commons/app.js 90.2 kB -175 B (0%)
./frontend/.nuxt/dist/client/commons/app.modern.js 78.5 kB -62 B (0%)
./frontend/.nuxt/dist/client/components/v-audio-details.js 1.81 kB -537 B (-23%) 🎉
./frontend/.nuxt/dist/client/components/v-content-report-form.js 3.31 kB -544 B (-14%) 👏
./frontend/.nuxt/dist/client/components/v-content-report-form.modern.js 3.19 kB -503 B (-14%) 👏
./frontend/.nuxt/dist/client/components/v-content-report-popover.js 3.78 kB +2.87 kB (+314%) 🆘
./frontend/.nuxt/dist/client/components/v-content-report-popover.modern.js 3.65 kB -501 B (-12%) 👏
./frontend/.nuxt/dist/client/components/v-image-details.js 1.42 kB -535 B (-27%) 🎉
./frontend/.nuxt/dist/client/components/v-radio.js 1 kB -536 B (-35%) 🎉
./frontend/.nuxt/dist/client/components/v-radio.modern.js 1.01 kB -496 B (-33%) 🎉
./frontend/.nuxt/dist/client/pages/audio/_id/index.js 6.09 kB -428 B (-7%)
./frontend/.nuxt/dist/client/pages/image/_id/index.js 7.05 kB -408 B (-5%)
./frontend/.nuxt/dist/client/pages/image/_id/report.js 4.99 kB +2.76 kB (+124%) 🆘
./frontend/.nuxt/dist/client/pages/image/_id/report.modern.js 4.74 kB -479 B (-9%)
./frontend/.nuxt/dist/client/vendors/app.js 64.7 kB +26 B (0%)
./frontend/.nuxt/dist/client/vendors/app.modern.js 63.8 kB -178 B (0%)
ℹ️ View Unchanged
Filename Size Change
./frontend/.nuxt/dist/client/234.js 272 B 0 B
./frontend/.nuxt/dist/client/234.modern.js 277 B 0 B
./frontend/.nuxt/dist/client/235.js 1.85 kB -1 B (0%)
./frontend/.nuxt/dist/client/commons/components/v-search-grid/pages/search.js 5.17 kB -1 B (0%)
./frontend/.nuxt/dist/client/commons/components/v-search-grid/pages/search.modern.js 5.61 kB +1 B (0%)
./frontend/.nuxt/dist/client/components/loading-icon.js 766 B +1 B (0%)
./frontend/.nuxt/dist/client/components/loading-icon.modern.js 772 B +2 B (0%)
./frontend/.nuxt/dist/client/components/table-sort-icon.js 514 B 0 B
./frontend/.nuxt/dist/client/components/table-sort-icon.modern.js 519 B 0 B
./frontend/.nuxt/dist/client/components/v-all-results-grid.js 5.76 kB 0 B
./frontend/.nuxt/dist/client/components/v-all-results-grid.modern.js 5.62 kB -9 B (0%)
./frontend/.nuxt/dist/client/components/v-audio-cell.js 422 B 0 B
./frontend/.nuxt/dist/client/components/v-audio-cell.modern.js 427 B +2 B (0%)
./frontend/.nuxt/dist/client/components/v-audio-details.modern.js 1.78 kB -3 B (0%)
./frontend/.nuxt/dist/client/components/v-audio-track-skeleton.js 1.02 kB 0 B
./frontend/.nuxt/dist/client/components/v-audio-track-skeleton.modern.js 1.02 kB -1 B (0%)
./frontend/.nuxt/dist/client/components/v-audio-track.js 5.33 kB +3 B (0%)
./frontend/.nuxt/dist/client/components/v-audio-track.modern.js 5.28 kB +1 B (0%)
./frontend/.nuxt/dist/client/components/v-back-to-search-results-link.js 636 B 0 B
./frontend/.nuxt/dist/client/components/v-back-to-search-results-link.modern.js 641 B +1 B (0%)
./frontend/.nuxt/dist/client/components/v-bone.js 688 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-bone.modern.js 691 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-box-layout.js 1.28 kB +1 B (0%)
./frontend/.nuxt/dist/client/components/v-box-layout.modern.js 1.28 kB +2 B (0%)
./frontend/.nuxt/dist/client/components/v-content-link.js 1.04 kB 0 B
./frontend/.nuxt/dist/client/components/v-content-link.modern.js 1.04 kB 0 B
./frontend/.nuxt/dist/client/components/v-content-page.js 524 B 0 B
./frontend/.nuxt/dist/client/components/v-content-page.modern.js 528 B 0 B
./frontend/.nuxt/dist/client/components/v-content-report-button.js 493 B 0 B
./frontend/.nuxt/dist/client/components/v-content-report-button.modern.js 496 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-copy-button.js 4 kB +2 B (0%)
./frontend/.nuxt/dist/client/components/v-copy-button.modern.js 4 kB +1 B (0%)
./frontend/.nuxt/dist/client/components/v-copy-license.js 2.09 kB -2 B (0%)
./frontend/.nuxt/dist/client/components/v-copy-license.modern.js 2.06 kB +1 B (0%)
./frontend/.nuxt/dist/client/components/v-dmca-notice.js 793 B 0 B
./frontend/.nuxt/dist/client/components/v-dmca-notice.modern.js 798 B -2 B (0%)
./frontend/.nuxt/dist/client/components/v-error-image.js 2.6 kB -4 B (0%)
./frontend/.nuxt/dist/client/components/v-error-image.modern.js 2.57 kB +3 B (0%)
./frontend/.nuxt/dist/client/components/v-error-section.js 372 B 0 B
./frontend/.nuxt/dist/client/components/v-error-section.modern.js 377 B 0 B
./frontend/.nuxt/dist/client/components/v-external-search-form.js 1.89 kB +2 B (0%)
./frontend/.nuxt/dist/client/components/v-external-search-form.modern.js 1.88 kB 0 B
./frontend/.nuxt/dist/client/components/v-external-source-list.js 899 B 0 B
./frontend/.nuxt/dist/client/components/v-external-source-list.modern.js 901 B +1 B (0%)
./frontend/.nuxt/dist/client/components/v-full-layout.js 1.56 kB -1 B (0%)
./frontend/.nuxt/dist/client/components/v-full-layout.modern.js 1.55 kB +1 B (0%)
./frontend/.nuxt/dist/client/components/v-grid-skeleton.js 1.62 kB +3 B (0%)
./frontend/.nuxt/dist/client/components/v-grid-skeleton.modern.js 1.62 kB -4 B (0%)
./frontend/.nuxt/dist/client/components/v-home-gallery.js 4.91 kB -5 B (0%)
./frontend/.nuxt/dist/client/components/v-home-gallery.modern.js 4.91 kB +5 B (0%)
./frontend/.nuxt/dist/client/components/v-homepage-content.js 1.77 kB 0 B
./frontend/.nuxt/dist/client/components/v-homepage-content.modern.js 1.74 kB +1 B (0%)
./frontend/.nuxt/dist/client/components/v-image-carousel.js 4.73 kB +2 B (0%)
./frontend/.nuxt/dist/client/components/v-image-carousel.modern.js 4.7 kB +4 B (0%)
./frontend/.nuxt/dist/client/components/v-image-cell.js 1.66 kB -1 B (0%)
./frontend/.nuxt/dist/client/components/v-image-cell.modern.js 1.65 kB +1 B (0%)
./frontend/.nuxt/dist/client/components/v-image-details.modern.js 1.41 kB +3 B (0%)
./frontend/.nuxt/dist/client/components/v-image-grid.js 3.9 kB +8 B (0%)
./frontend/.nuxt/dist/client/components/v-image-grid.modern.js 3.78 kB +2 B (0%)
./frontend/.nuxt/dist/client/components/v-license-tab-panel.js 642 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-license-tab-panel.modern.js 649 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-load-more.js 846 B 0 B
./frontend/.nuxt/dist/client/components/v-load-more.modern.js 741 B +1 B (0%)
./frontend/.nuxt/dist/client/components/v-media-license.js 827 B 0 B
./frontend/.nuxt/dist/client/components/v-media-license.modern.js 835 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-media-reuse.js 2.68 kB 0 B
./frontend/.nuxt/dist/client/components/v-media-reuse.modern.js 2.65 kB -2 B (0%)
./frontend/.nuxt/dist/client/components/v-media-tag.js 416 B 0 B
./frontend/.nuxt/dist/client/components/v-media-tag.modern.js 421 B +1 B (0%)
./frontend/.nuxt/dist/client/components/v-modal.js 1 kB -1 B (0%)
./frontend/.nuxt/dist/client/components/v-modal.modern.js 995 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-no-results.js 814 B 0 B
./frontend/.nuxt/dist/client/components/v-no-results.modern.js 815 B 0 B
./frontend/.nuxt/dist/client/components/v-related-audio.js 1.29 kB +2 B (0%)
./frontend/.nuxt/dist/client/components/v-related-audio.modern.js 1.29 kB 0 B
./frontend/.nuxt/dist/client/components/v-related-images.js 1.1 kB +3 B (0%)
./frontend/.nuxt/dist/client/components/v-related-images.modern.js 1.1 kB -3 B (0%)
./frontend/.nuxt/dist/client/components/v-report-desc-form.js 976 B +1 B (0%)
./frontend/.nuxt/dist/client/components/v-report-desc-form.modern.js 981 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-row-layout.js 1.68 kB +2 B (0%)
./frontend/.nuxt/dist/client/components/v-row-layout.modern.js 1.68 kB +2 B (0%)
./frontend/.nuxt/dist/client/components/v-scroll-button.js 825 B +1 B (0%)
./frontend/.nuxt/dist/client/components/v-scroll-button.modern.js 829 B 0 B
./frontend/.nuxt/dist/client/components/v-search-grid.js 6.87 kB -2 B (0%)
./frontend/.nuxt/dist/client/components/v-search-grid.modern.js 6.22 kB -1 B (0%)
./frontend/.nuxt/dist/client/components/v-search-results-title.js 618 B -2 B (0%)
./frontend/.nuxt/dist/client/components/v-search-results-title.modern.js 621 B +1 B (0%)
./frontend/.nuxt/dist/client/components/v-server-timeout.js 299 B -1 B (0%)
./frontend/.nuxt/dist/client/components/v-server-timeout.modern.js 303 B 0 B
./frontend/.nuxt/dist/client/components/v-sketch-fab-viewer.js 1.01 kB -1 B (0%)
./frontend/.nuxt/dist/client/components/v-sketch-fab-viewer.modern.js 912 B 0 B
./frontend/.nuxt/dist/client/components/v-snackbar.js 1.16 kB 0 B
./frontend/.nuxt/dist/client/components/v-snackbar.modern.js 1.17 kB -1 B (0%)
./frontend/.nuxt/dist/client/components/v-sources-table.js 15.2 kB -1 B (0%)
./frontend/.nuxt/dist/client/components/v-sources-table.modern.js 15.2 kB 0 B
./frontend/.nuxt/dist/client/components/v-warning-suppressor.js 306 B 0 B
./frontend/.nuxt/dist/client/components/v-warning-suppressor.modern.js 311 B 0 B
./frontend/.nuxt/dist/client/pages/about.js 1.39 kB -1 B (0%)
./frontend/.nuxt/dist/client/pages/about.modern.js 1.4 kB 0 B
./frontend/.nuxt/dist/client/pages/audio/_id/index.modern.js 5.9 kB -5 B (0%)
./frontend/.nuxt/dist/client/pages/external-sources.js 1.55 kB 0 B
./frontend/.nuxt/dist/client/pages/external-sources.modern.js 1.56 kB +1 B (0%)
./frontend/.nuxt/dist/client/pages/feedback.js 1.34 kB -1 B (0%)
./frontend/.nuxt/dist/client/pages/feedback.modern.js 1.34 kB +1 B (0%)
./frontend/.nuxt/dist/client/pages/image/_id/index.modern.js 6.77 kB -2 B (0%)
./frontend/.nuxt/dist/client/pages/index.js 7.03 kB -4 B (0%)
./frontend/.nuxt/dist/client/pages/index.modern.js 6.96 kB +6 B (0%)
./frontend/.nuxt/dist/client/pages/preferences.js 1.3 kB -1 B (0%)
./frontend/.nuxt/dist/client/pages/preferences.modern.js 1.3 kB +2 B (0%)
./frontend/.nuxt/dist/client/pages/privacy.js 1.23 kB -3 B (0%)
./frontend/.nuxt/dist/client/pages/privacy.modern.js 1.24 kB 0 B
./frontend/.nuxt/dist/client/pages/search-help.js 1.6 kB +2 B (0%)
./frontend/.nuxt/dist/client/pages/search-help.modern.js 1.58 kB +1 B (0%)
./frontend/.nuxt/dist/client/pages/search.js 2.2 kB -1 B (0%)
./frontend/.nuxt/dist/client/pages/search.modern.js 2.04 kB +3 B (0%)
./frontend/.nuxt/dist/client/pages/search/audio.js 3.67 kB +3 B (0%)
./frontend/.nuxt/dist/client/pages/search/audio.modern.js 3.55 kB -6 B (0%)
./frontend/.nuxt/dist/client/pages/search/image.js 545 B -1 B (0%)
./frontend/.nuxt/dist/client/pages/search/image.modern.js 547 B -2 B (0%)
./frontend/.nuxt/dist/client/pages/search/index.js 443 B 0 B
./frontend/.nuxt/dist/client/pages/search/index.modern.js 447 B -1 B (0%)
./frontend/.nuxt/dist/client/pages/search/model-3d.js 242 B -1 B (0%)
./frontend/.nuxt/dist/client/pages/search/model-3d.modern.js 246 B 0 B
./frontend/.nuxt/dist/client/pages/search/search-page.types.js 266 B 0 B
./frontend/.nuxt/dist/client/pages/search/search-page.types.modern.js 270 B 0 B
./frontend/.nuxt/dist/client/pages/search/video.js 239 B 0 B
./frontend/.nuxt/dist/client/pages/search/video.modern.js 243 B 0 B
./frontend/.nuxt/dist/client/pages/sources.js 1.51 kB 0 B
./frontend/.nuxt/dist/client/pages/sources.modern.js 1.52 kB 0 B
./frontend/.nuxt/dist/client/runtime.js 2.71 kB +1 B (0%)
./frontend/.nuxt/dist/client/runtime.modern.js 2.72 kB 0 B

compressed-size-action

@github-actions
Copy link

Full-stack documentation: https://docs.openverse.org/_preview/2135

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.

@obulat obulat mentioned this pull request May 19, 2023
8 tasks
@obulat obulat force-pushed the remove/module_@nuxtjs/svg branch from 0241fd5 to 66e117e Compare May 19, 2023 12:12
@obulat obulat changed the base branch from main to fix/header_home_link May 19, 2023 12:32
@obulat obulat force-pushed the remove/module_@nuxtjs/svg branch from 66e117e to 64bf705 Compare May 19, 2023 13:42
@obulat obulat added 🟨 priority: medium Not blocking but should be addressed soon 💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users and removed 🧱 stack: documentation Related to Sphinx documentation labels May 19, 2023
@obulat obulat self-assigned this May 19, 2023
@github-actions github-actions bot added the 🧱 stack: documentation Related to Sphinx documentation label May 19, 2023
@obulat obulat force-pushed the fix/header_home_link branch 2 times, most recently from e87d12f to ca6191e Compare May 19, 2023 14:44
@obulat obulat force-pushed the remove/module_@nuxtjs/svg branch from 299fb93 to dd84531 Compare May 19, 2023 14:46
@obulat obulat marked this pull request as ready for review May 19, 2023 17:10
@obulat obulat requested a review from a team as a code owner May 19, 2023 17:10
@openverse-bot openverse-bot moved this from In progress to Needs review in Openverse PRs May 19, 2023
@obulat obulat marked this pull request as draft May 20, 2023 03:02
@openverse-bot openverse-bot moved this from Needs review to In progress in Openverse PRs May 20, 2023
@obulat obulat force-pushed the fix/header_home_link branch 3 times, most recently from 5a0ec34 to 375c672 Compare May 23, 2023 12:48
@obulat obulat force-pushed the remove/module_@nuxtjs/svg branch from dd84531 to fc4e441 Compare May 23, 2023 12:56
@obulat obulat changed the base branch from fix/header_home_link to main May 23, 2023 14:26
@obulat obulat force-pushed the remove/module_@nuxtjs/svg branch 2 times, most recently from 27b08a0 to aeba68b Compare May 25, 2023 04:17
@obulat obulat marked this pull request as ready for review May 25, 2023 16:16
@openverse-bot openverse-bot moved this from In progress to Needs review in Openverse PRs May 25, 2023
Copy link
Contributor

@sarayourfriend sarayourfriend left a comment

Choose a reason for hiding this comment

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

This is great 🎉

@obulat obulat force-pushed the remove/module_@nuxtjs/svg branch 2 times, most recently from 99f4c1d to 5e102b7 Compare May 26, 2023 08:12
@obulat obulat requested a review from zackkrida May 26, 2023 10:07
Copy link
Contributor

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

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

LGTM, but I have a few questions!

aria-hidden="true"
focusable="false"
>
<use :href="`${Oops}#oops`" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this href have been deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the <VSvg> component handles finding the hrefs now, and we do not use the id of the inner SVG path. We only need to pass the name of the file to use it as an SVG.

frontend/src/components/VSvg/VSvg.vue Outdated Show resolved Hide resolved
Openverse PRs automation moved this from Needs review to Reviewer approved May 26, 2023
Copy link
Contributor

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

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

(don't want to block a merge though)

@obulat obulat force-pushed the remove/module_@nuxtjs/svg branch from 21e2665 to 24b8d1a Compare May 29, 2023 11:37
@obulat obulat merged commit a00149e into main May 29, 2023
38 checks passed
Openverse PRs automation moved this from Reviewer approved to Merged! May 29, 2023
@obulat obulat deleted the remove/module_@nuxtjs/svg branch May 29, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: documentation Related to Sphinx documentation 🧱 stack: frontend Related to the Nuxt frontend
Projects
Archived in project
Openverse PRs
  
Merged!
Development

Successfully merging this pull request may close these issues.

Use svg-sprite module for loading all SVG files
3 participants