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

feat(Images): Add asset image support for wysiwyg node type #40

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mjoellnier
Copy link

Hey there!

I added a recognition of Image tags in the wysiwyg value. So when an cockpit asset is used the image url gets replaced by the fetched one. External images stay untouched.

👋

Copy link
Collaborator

@WhippetsAintDogs WhippetsAintDogs left a comment

Choose a reason for hiding this comment

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

Hi @mjoellnier ! Thanks for your contribution and sorry for the delays.

Almost everything seems fine, I've added refactor suggestions to apply before integrating into master (for a more consistent code in regard to the rest of the project).

The only problem remaining is that currently, in order for your code to work, the inserted image in the Wysiwyg has to be included at the same time by some other nodes of type Asset, Image or Gallery because it isn't pulled locally otherwise (and in such cases, the expression images[sources[index]] would be undefined). Have a look at the normalizeResources method of the CockpitService class in order to understand how to eventually add and link a normalizeNodeItemWysiwygs method.

Also, could you:

  • squash your two commits into one (keeping the last one's name);
  • rebase onto latest master;
  • bump package.json version to 1.1.2;
  • update package.json description with: "This is a Gatsby version 2.x.x source plugin that feeds the GraphQL tree with Cockpit Headless CMS collections and singletons data. Actually, it supports querying raw texts (and any trivial field types), Markdown, Wysiwyg, images, galleries, assets, sets, repeaters, layout(-grid)s (currently only without nested images/assets), objects, linked collections and internationalization.";
  • update readme.md description on line 5 with: Actually, it supports querying raw texts (and any trivial field types), Markdown, Wysiwyg, images, galleries, assets, sets, repeaters, layout(-grid)s (currently only without nested images/assets), objects, linked collections and internationalization.;
  • update readme.md by adding that section below the #### Markdowns section:
#### Wysiwygs

Wysiwyg fields (or `What you see is what you get`) works the same way as Markdown fields, but instead of accessing an HTML equivalent through the `value → childMarkdownRemark → html` attribute, it's accessible straight through the `value` attribute.

src/helpers.js Outdated Show resolved Hide resolved
src/helpers.js Outdated Show resolved Hide resolved
@mjoellnier
Copy link
Author

Hi @WhippetsAintDogs
Thanks a lot for your feedback 🤗 I will have a look at it as soon as possible and will let you know!

Changed img tag recognition and Url vs asset

Test commit

Revert "Test commit"

This reverts commit 084c855.

Implemented review feedback
@mjoellnier
Copy link
Author

Hi @WhippetsAintDogs

thank you again for your feedback! I implemented the missing parts and added your refactoring suggestions. So please recheck the code - I tested it locally and it works well with images that are only added via WYSIWYG.

Take care 👋

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