-
Notifications
You must be signed in to change notification settings - Fork 528
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
feat(experiences): implement custom element #6364
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit c88b163:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good to have both options, probably we'll want to converge on only custom elements later but this gives us options. Thanks!
this.disconnectedCallback(); | ||
this.connectedCallback(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for later: the order should be fetch -> remove -> add, not remove -> fetch -> add, but I see how this current code is much simpler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be simple, I could wrap this into fetchConfiguration(newValue).then
and it would pick up the cached configuration in connectedCallback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also good for me :)
class AlgoliaExperience extends HTMLElement { | ||
static observedAttributes = ['experience-id']; | ||
|
||
widgets: IndexWidget[] = []; | ||
|
||
connectedCallback() { | ||
const experienceId = this.getAttribute('experience-id'); | ||
if (!experienceId) { | ||
const id = this.getAttribute('experience-id'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would there be less repetition if we call the attribute data-id
? can swap before the GA still
FX-3008
Summary
To allow usage within an SPA, be it completely client-side rendered or not (with Next.js for example), we need to provide a way for an experience to be loaded at any time.
Result
Created a custom element that fetches its own configuration and renders. Code looks a bit awkward since I reused functions meant for multiple configurations.
We should probably implement some caching later for configurations.