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]: TalonWrapper needs Component injection #2521

Open
2 of 8 tasks
fooman opened this issue Jul 1, 2020 · 5 comments
Open
2 of 8 tasks

[feature]: TalonWrapper needs Component injection #2521

fooman opened this issue Jul 1, 2020 · 5 comments
Labels

Comments

@fooman
Copy link
Contributor

fooman commented Jul 1, 2020

Is your feature request related to a problem? Please describe.
The extension points are currently limited when trying to inject additional content/components.

Describe the solution you'd like
Currently for the useProductFullDetails we can wrap this function

    const talonProps = useProductFullDetail({
        addConfigurableProductToCartMutation: ADD_CONFIGURABLE_MUTATION,
        addSimpleProductToCartMutation: ADD_SIMPLE_MUTATION,
        createCartMutation: CREATE_CART_MUTATION,
        getCartDetailsQuery: GET_CART_DETAILS_QUERY,
        product
    });

unfortunately it is a bit of dead end if we want to have additional content rendered. Consider trying to for example add a video component into the below:

    return (
        <Fragment>
            {breadcrumbs}
            <Form className={classes.root}>
[...]
            </Form>
        </Fragment>

there doesn't seem to be an option to get from the talon prop into the Fragment. Ideally we would have something like

    const {
        breadcrumbCategoryId,
        handleAddToCart,
        handleSelectionChange,
        handleSetQuantity,
        isAddToCartDisabled,
        mediaGalleryEntries,
        productDetails,
        quantity,
        extraContentStart,
        extraContentEnd,
        extraFormContent
    } = talonProps;
    return (
        <Fragment>
            {extraContentStart}
            {breadcrumbs}
            <Form className={classes.root}>
[...]
                        {extraFormContent}
            </Form>
            {extraContentEnd}
        </Fragment>

Describe alternatives you've considered
An alternative could be to create an additional venia-ui target for productFullDetail.

Please let us know what packages this feature is in regards to:

  • venia-concept
  • venia-ui
  • pwa-buildpack
  • peregrine
  • pwa-devdocs
  • upward-js
  • upward-spec
  • create-pwa
@fooman fooman added the enhancement New feature or request label Jul 1, 2020
@m2-assistant
Copy link

m2-assistant bot commented Jul 1, 2020

Hi @fooman. Thank you for your report.
To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this


@Jordaneisenburger
Copy link
Member

This might be related with #2281

@fooman
Copy link
Contributor Author

fooman commented Jul 3, 2020

@Jordaneisenburger Definitely related. I see #2281 as the use case where one is already customising venia-ui. The above would be trying to inject content completely via a separate package without the need to customise venia-ui nor peregrine.

@sirugh
Copy link
Contributor

sirugh commented May 17, 2021

@magento export issue to JIRA project PWA as Story

@github-jira-sync-bot
Copy link

✅ Jira issue https://jira.corp.magento.com/browse/PWA-1773 is successfully created for this GitHub issue.

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

No branches or pull requests

4 participants