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

[FEATURE] Calls to Action inside text elements should be parsed #649

Closed
3 tasks
Rigo-m opened this issue Apr 24, 2020 · 2 comments · Fixed by #744
Closed
3 tasks

[FEATURE] Calls to Action inside text elements should be parsed #649

Rigo-m opened this issue Apr 24, 2020 · 2 comments · Fixed by #744

Comments

@Rigo-m
Copy link
Collaborator

Rigo-m commented Apr 24, 2020

Description

Calls to Action (which are called link in the Shopware editor) inside text elements should be parsed to become a storefront-ui CTA atom.

Acceptance criteria

  • CTA that comes from a text element should be parsed into a storefront-ui cta (or equivalent)
  • It should be made in a proper "safe" way that prevents possible insecurities
  • A CTA added by code and one parsed from within a text element should come from the same component and be equivalent
@Rigo-m
Copy link
Collaborator Author

Rigo-m commented May 7, 2020

There are a couple of ways to achieve this.
I want to create a pull request but I feel that before going to one or another implementation I need to assess the implementation costs.

To parse components inside a string you either have to:

  • Leverage <component> template capabilities by passing an object to the is prop. This requires full vue build with runtimeCompiler option
  • Parsing the rawHtml with an Html to AST parser and create our own render function

First option is way more convenient, faster to implement and readable (especially for those who don't have much familiarity with render functions), although bundle size would be bigger.
The second option is less readable but also doesn't impact the bundle size that much (there are AST parsers which are very lightweight and since we know that the rawHtml is fairly simple, we don't need complicated ones).

Performance-wise, I don't think that there is much difference between the two options, since the HTML that gets parsed is not so much complex (IMHO).

I'd like to hear something from you guys so I can make the pull request happen.

@patzick
Copy link
Collaborator

patzick commented May 11, 2020

@Rigo-m great input
I believe both solutions are acceptable here, but using render function here would be a nicer one in a long term, as for example all internal links should be presented as <nuxt-link> and same could be used to detect CTA atom (it's already have a link inside, but there can be more links in text). Probably CTA component should be shown when we have display as a button in CMS, so the link has btn btn-primary classes inside <a>. So this logic may get complicated in time as well, so having smaller bundle would be helpful here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants