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

restoring social share #54

Merged
merged 11 commits into from
Dec 31, 2021
Merged

Conversation

thescientist13
Copy link
Contributor

@thescientist13 thescientist13 commented Dec 5, 2021

Related Issue

resolves #39

Summary of Changes

  1. Restored social sharing using web-social-share
  2. Swapped Google with Pinterest (since Google+ is no more)

Screen Shot 2021-12-23 at 6 40 17 PM
Screen Shot 2021-12-10 at 7 43 44 PM

So there seem to be some issues(1, 2 with Stencil that made progress on this a bit challenging, but with a little tweaking to a fork, I got things working by:

  1. pointing the module field to dist/websocialshare/websocialshare.esm.js instead of dist/esm/index.js, which was just resolving to to an empty file
  2. Unfortunately things break down a bit more when building for production, and it looks like some of the other _dist/ files need to be there? My guess is Rollup doesn't know how to bundle variables in a dynamic import?
    import(`./${n}.entry.js`).then((e=>(D.set(n,e),e[t])),B)
    This was solved by adding a Rollup plugin specifically for handling variables in dynamic imports.
Tried manually copying those files and now getting this? So... progress? ![Screen Shot 2021-12-10 at 8 10 27 PM](https://user-images.githubusercontent.com/895923/145658475-d845a65c-a6c3-4ef1-bdde-a110ab613882.png)

TODOs

  1. Fix for production build command
  2. Get the share into its place on the left top of the page
  3. Get upstream fix from Greenwood - dependencies that reference more than one directory level of traversal (../) in their ESM specifiers do not resolve correctly ProjectEvergreen/greenwood#820

@thescientist13 thescientist13 added the feature New feature or request label Dec 5, 2021
@thescientist13 thescientist13 self-assigned this Dec 5, 2021
@netlify
Copy link

netlify bot commented Dec 5, 2021

✔️ Deploy Preview for practical-fermat-fa2c48 ready!

🔨 Explore the source changes: 92c5562

🔍 Inspect the deploy log: https://app.netlify.com/sites/practical-fermat-fa2c48/deploys/61cf87f648c99f0008636e52

😎 Browse the preview: https://deploy-preview-54--practical-fermat-fa2c48.netlify.app/

@thescientist13 thescientist13 added needs upstream help wanted Extra attention is needed question Further information is requested labels Dec 11, 2021
@thescientist13 thescientist13 changed the title WIP of integrating and restoring social share [WIP] integrating and restoring social share Dec 21, 2021
@thescientist13 thescientist13 removed needs upstream help wanted Extra attention is needed question Further information is requested labels Dec 23, 2021
@thescientist13 thescientist13 marked this pull request as ready for review December 23, 2021 23:41
@thescientist13 thescientist13 changed the title [WIP] integrating and restoring social share restoring social share Dec 23, 2021
@thescientist13 thescientist13 force-pushed the feature/issue-39-restore-social-share branch from d1687fe to bcaafe9 Compare December 24, 2021 00:14
@thescientist13 thescientist13 merged commit 99ef6ed into main Dec 31, 2021
@thescientist13 thescientist13 deleted the feature/issue-39-restore-social-share branch December 31, 2021 22:51
@thescientist13 thescientist13 linked an issue Jan 5, 2022 that may be closed by this pull request
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

restore social share implementation for details pages (artist, album, event) Migrate Components and Services
1 participant