feat(atomic): expose idiomatic react templating functionality #1677
Conversation
|
Thanks for your contribution @olamothe !
|
|
Pull Request Report PR Title ✅ Title follows the conventional commit spec. Bundle Size
|
| @@ -182,7 +197,9 @@ export class AtomicResultList implements InitializableComponent { | |||
|
|
|||
| private buildListResults() { | |||
| return this.resultListState.results.map((result) => { | |||
| const template = this.getTemplate(result); | |||
| const content = this.renderingFunction | |||
There was a problem hiding this comment.
More simple than I expected 😄
| @@ -90,6 +99,12 @@ export class AtomicResultList implements InitializableComponent { | |||
| */ | |||
| @Prop() image: ResultDisplayImageSize = 'icon'; | |||
|
|
|||
| private renderingFunction?: (result: Result) => HTMLElement = undefined; | |||
|
|
|||
| @Method() public setRenderFunction(render: (result: Result) => HTMLElement) { | |||
There was a problem hiding this comment.
Should we set this as @internal ?
There was a problem hiding this comment.
Is this only for documentation ?
Or does it have any other impact to do that ?
There was a problem hiding this comment.
Speaking of docs, a line to explain what this function is for would be useful, especially since it's not directly used inside Atomic
There was a problem hiding this comment.
Whether we make it public/documented or not, this will need to be maintained since we use it in atomic-react and it's likely going to be used in other frameworks. I have a slight preference for documenting it.
There was a problem hiding this comment.
@jpmarceau / @npushkarskii : Small review for the comment here please !
Thanks ! 🙏
| @@ -46,6 +46,7 @@ export const config: Config = { | |||
| componentCorePackage: '@coveo/atomic', | |||
| proxiesFile: '../atomic-react/src/components/stencil-generated/index.ts', | |||
| includeDefineCustomElements: true, | |||
| excludeComponents: ['atomic-result-template', 'atomic-field-condition'], | |||
There was a problem hiding this comment.
A bit unrelated, but maybe the atomic-external should be investigated and perhaps excluded too if it makes thing too complicated (the react context should remove the necessity for it)
|
|
||
| function MyTemplate(result: Result) { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
is this like a Fragment?
There was a problem hiding this comment.
It's a way to return a "single root" for a JSX function, without the need for any extra DOM node.
| @@ -90,6 +99,12 @@ export class AtomicResultList implements InitializableComponent { | |||
| */ | |||
| @Prop() image: ResultDisplayImageSize = 'icon'; | |||
|
|
|||
| private renderingFunction?: (result: Result) => HTMLElement = undefined; | |||
|
|
|||
| @Method() public setRenderFunction(render: (result: Result) => HTMLElement) { | |||
There was a problem hiding this comment.
Speaking of docs, a line to explain what this function is for would be useful, especially since it's not directly used inside Atomic
| @@ -0,0 +1,21 @@ | |||
| diff --git a/node_modules/@stencil/react-output-target/react-component-lib/createComponent.tsx b/node_modules/@stencil/react-output-target/react-component-lib/createComponent.tsx | |||
There was a problem hiding this comment.
Cool approach to work-around the issue. It would be nice to open a PR for the stencil package too,
packages/atomic/src/components/atomic-result-list/atomic-result-list.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Nikita Pushkarskii <[email protected]>
As discussed in scrum, the goal is to expose a more natural React pattern to render result templates VS the Web Component/HTML way.
Changes included in this PR:
atomic-result-templateandatomic-field-conditionfrom the output target (These components will no longer appear under@coveo/atomic-reactsetRenderFunctionmethod to the core Atomic result list web component, which accept a function that receive a result as a parameter, and has to return an HTMLElementAtomicResultList, with one additional prop:template, which accept a result as a parameter and must return a JSX.Element. It then does the necessary work to change this to an HTMLElement, which is what Atomic expects.Also included in this PR:
A patch to fix an issue with the react output plugin, which does not properly
camelToDashCaseHTML attributes.https://coveord.atlassian.net/browse/KIT-1336