Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

Move image handler into pipeline step before "meta" #706

Closed
rofe opened this issue May 7, 2020 · 10 comments · Fixed by #737
Closed

Move image handler into pipeline step before "meta" #706

rofe opened this issue May 7, 2020 · 10 comments · Fixed by #737
Labels
enhancement New feature or request released

Comments

@rofe
Copy link
Contributor

rofe commented May 7, 2020

Helix Pages now uses context.content.image in the metadata, but in case of Azure blob store images it unfortunately doesn't contain the /hlx_* URL.

@tripodsan suggested to move the image handler, which is currently rewriting blob store links on the VDOM as part of the "html" step, into its own "image" step and insert it before "meta", so the image URLs are already rewritten in the mdast when metadata is being extracted.

@rofe
Copy link
Contributor Author

rofe commented May 7, 2020

An alternative approach would be to move the "meta" step after the "html" step (thereby slightly breaking the extension point API) and extract the metadata from the VDOM.

@tripodsan
Copy link
Contributor

@trieloff was there a specific reason you added the image-helixalizer as part of the mdast->VDOM transformation and not as an operation on the mdast ?

@trieloff
Copy link
Contributor

trieloff commented May 8, 2020

If I remember correctly, you said that you wouldn’t do this in word2md, so that the markdown would stay consistent. I might have concluded that this was a transformation that is specific to the HTML conversion.

We can still pull this out into a separate pipeline step.

@tripodsan
Copy link
Contributor

tripodsan commented May 8, 2020

If I remember correctly, you said that you wouldn’t do this in word2md,

yes. not in word2md.

we could:

  • (add it as extra step, operating on the VDOM)
  • move the get-metadata after the html transformation and make it work on vdom

or

  • add it as extra step, but operate on mdast
  • move it before the get-metadata step

@tripodsan
Copy link
Contributor

I might have concluded that this was a transformation that is specific to the HTML conversion.

Right. But all other 'special' operations are individual steps.

@trieloff
Copy link
Contributor

trieloff commented May 8, 2020

I very much prefer your second option.

  • MDAST is cleaner than HTML
  • Metadata detection works on MDAST and I wouldn't want to port it to working on HTML

@tripodsan
Copy link
Contributor

second option.

?

@trieloff
Copy link
Contributor

trieloff commented May 8, 2020

or

  • add it as extra step, but operate on mdast
  • move it before the get-metadata step

^^ that second option

@rofe
Copy link
Contributor Author

rofe commented May 8, 2020

Or the one already mentioned in the description ^^ 😉

@adobe-bot
Copy link

🎉 This issue has been resolved in version 7.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants