Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Create a new Target for RichContentTypes (data-content-type=) #2968 #3260

Closed
wants to merge 4 commits into from
Closed

Conversation

0m3r
Copy link
Contributor

@0m3r 0m3r commented Jul 8, 2021

Description

It gets the possibility for an extension to add custom content type renderers.

https://github.com/magento/pwa-studio/blob/develop/packages/pagebuilder/lib/config.js#L29

Related Issue

#2130
#2131
#2968

Closes #2968

Acceptance

@sirugh
@zetlen

Verification Steps

intercept.js

    targets
        .of('@magento/venia-ui')
        .richContentTypes.tap(contentTypes => {
            contentTypes.add({
                contentType: 'XSlider',
                importPath: require.resolve(
                    "../lib/components/XSlider/ContentTypes/XSlider/index.js"
                )
            });
        });

lib/components/XSlider/ContentTypes/XSlider/index.js

import Component from './xslider';
import configAggregator from './configAggregator';

export default {
    name: 'xslider',
    configAggregator,
    component: Component
}

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Jul 8, 2021

Fails
🚫

node` failed.

🚫 A version label is required. A maintainer must add one.
🚫

The following file(s) were not formatted with prettier. Make sure to execute yarn run prettier locally prior to committing.

packages/venia-ui/lib/components/RichContent/richContent.js
packages/venia-ui/lib/targets/RichContentTypeList.js
Messages
📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Log

ERROR ON TASK: prettierCheck


Error:  Danger had errors running. See message(s) above for more details.
danger-results://tmp/danger-results.json

Generated by 🚫 dangerJS against ddda01a

@larsroettig larsroettig self-requested a review July 28, 2021 15:14
@@ -3,6 +3,8 @@ import { useStyle } from '../../classify';
import defaultClasses from './richContent.css';
import { shape, string } from 'prop-types';
import richContentRenderers from './richContentRenderers';
import { setContentTypeConfig } from '@magento/pagebuilder/lib/config';
Copy link
Member

Choose a reason for hiding this comment

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

We can not couple our theme with a standalone extension. From my point of view pagebuilder should have an extension point to inject local components into the page builder. There we need to mutch better API also maybe a Partner/Agency would overwrite the slider via this extension point. Currently, it is not possible to register a target on page builder it self so we need to create a story in our backlog.

@larsroettig
Copy link
Member

❌ Needs SPIKE ticket for an API de# the core

Thank you for your contribution @0m3r unfortunately I cannot approve it because cycle dependency page builder is not a must-have for venia to work correctly.

If you want, you can create a new PR with no cycle. From my point of view, the core team need to handle it with a Spike or Poc.

Kind regards,

Lars

@0m3r
Copy link
Contributor Author

0m3r commented Jul 29, 2021

Ok I will try to create a new PR

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

Successfully merging this pull request may close these issues.

[feature]: Extensibility point (new target) to Pagebuilder setContentTypeConfig
3 participants