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

Generative AI Page Designer Component #1698

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Generative AI Page Designer Component #1698

wants to merge 11 commits into from

Conversation

kevinxh
Copy link
Collaborator

@kevinxh kevinxh commented Mar 13, 2024

I'm looking to do ship nightly releases for the Gen AI PD component to make this feature avaliable to the pilot customers. This way we can still iterate on the API without worrying about making breaking changes until we beta/GA this feature.

TODO - I need to figure out how to include this in a nightly build.

* move code to commerce-sdk-react

* add list of egpt components

* fix ts issue

* add einstein assisted component validation

* minor fixes

* fix typing

* try to bump bundlesize
@kevinxh kevinxh requested a review from a team as a code owner March 13, 2024 23:49
@@ -97,7 +97,7 @@
},
{
"path": "build/vendor.js",
"maxSize": "320 kB"
"maxSize": "370 kB"
Copy link
Collaborator Author

@kevinxh kevinxh Mar 14, 2024

Choose a reason for hiding this comment

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

there is a backlog ticket to code split the react-jsx-parser lib.

@@ -44,11 +48,15 @@ export const usePageContext = () => {
* @param {PageProps} props
* @param {Page} props.region - The page designer page data representation.
* @param {ComponentMap} props.components - A mapping of typeId's to react components representing the type.
* @param {JsxParserComponents} [props.jsxParserComponents] - An array of react components that the jsx parser can use, optional.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, i don't like the name of this prop... Any other ideas?

const {data, ...rest} = component
const {code} = data || {}
const isEinsteinAssistedComponent = !!code
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned before this should look at the component.typeId to determine if this is in fact the EGPT component. Otherwise this can/will get triggered by unrelated components that have this (commonly-named) property.

const isEinsteinAssistedComponent = !!code
let instance = <ComponentNotFound {...rest} {...data} />

useEffect(() => {
Copy link
Contributor

@clavery clavery Mar 14, 2024

Choose a reason for hiding this comment

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

Is there a strong reason the eGPT component needs to be elevated to special status in this "container" component? Can it not just be another PD "ComponentClass" that is added to the ComponentMap of the page?

That would give you a few benefits:

  • You can move this logic out of this file including the heavy react-jsx-parser dependency. It would only be brought in if the customer elects to add it to their component map and wouldn't affect everyones (or the default) bundle size. Right now this is brought in once they use this version of the SDK.
  • Avoids adding new props to the Page wrapper like JsxParserComponents. Instead customers add the egpt assisted component where they are already used to
  • Lets folks use the einstein assisted component directly (like in their own page designer implementations: like my own which is slightly different in it's wrappers)
    • Related to this you could, in theory if you wanted to, ship the eGPT component as a separate package instead of in commerce-sdk-react
  • In theory customers can use the egpt editor in their own components that might have other attributes. So this would support that since they could dispatch the JSX part to be handled by the EGPT component.

Since you still need to know the "JsxParserComponents" instead of passing in the component class itself you can have a higher-order function create it. i.e. Thinking customers could initialize their page designer based page like this which would look very familiar

import {usePage} from '@salesforce/commerce-sdk-react'
import {Page} from '@salesforce/commerce-sdk-react/components'
import {makeEinsteinAssistedComponent} from '@salesforce/commerce-sdk-react/components/egpt'

import {ImageTile, ImageWithText} from '../../page-designer/assets'
// import chakra components, ideally define this constant in 1 shared place
const DEFAULT_EGPT_JSX_COMPONENTS = {
    // bunch of chakra components
    
    // add table support
    TableContainer,
    Table,
    Thead,
    Tbody,
    Th,
    Td,
    Tr,
    TableCaption,
}

import {
    Carousel
} from '../../page-designer/layouts'

const PAGEDESIGNER_TO_COMPONENT = {
    'commerce_assets.photoTile': ImageTile,
    'commerce_assets.imageAndText': ImageWithText,
    'commerce_layouts.carousel': Carousel,
    'einstein.einsteinAssisted': makeEinsteinAssistedComponent({
        ...DEFAULT_EGPT_JSX_COMPONENTS,
        SomeOtherComponent
    })
}

const PageViewer = () => {
    // ...
    return (
        <Box layerStyle={'page'}>
            <Page page={page} components={PAGEDESIGNER_TO_COMPONENT} />
        </Box>
    )
}

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.

None yet

4 participants