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

feat(autocomplete-js): expose default render function in render #878

Closed
wants to merge 1 commit into from

Conversation

sarahdayan
Copy link
Member

@sarahdayan sarahdayan commented Jan 26, 2022

Summary

This exposes the render function from our internal Preact dependency to autocomplete.render.

When users implement autocomplete.render themselves (e.g., to customize the panel, create two-column layouts, etc.) they need to manually render the component tree. For now, this means they must import a render function themselves, as demonstrated here.

This isn't ideal because it means these users must add a VDOM dependency to their project. For users who don't use JSX, this can be a turnoff.

We already use Preact internally, so we can provide render to the user, the same way we provide createElement and Fragment.

Usage

Vanilla JavaScript

autocomplete({
  // ...
  render({ sections, createElement, render }, root) {
    render(
      createElement('div', {
        className: 'aa-PanelLayout aa-Panel--scrollable',
      }, sections),
      root
    );
  },
});

JSX

autocomplete({
  // ...
  render({ sections, render }, root) {
    render(
      <div className="aa-PanelLayout aa-Panel--scrollable">{sections}</div>,
      root
    );
  },
});

@sarahdayan sarahdayan requested review from francoischalifour, a team and dhayab and removed request for a team January 26, 2022 15:09
@codesandbox-ci
Copy link

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.

@Haroenv
Copy link
Contributor

Haroenv commented Jan 26, 2022

I actually think render should not be needed, but we should instead accept elements and add them into the tree (instead of rendering inside the tree).

Wouldn't this be part of an "accept html in autocomplete" RFC?

Overall I agree that the default createElement should be available if you don't add it as a dependency!

@sarahdayan
Copy link
Member Author

@Haroenv I added this because while updating the docs with HTML alternatives, I realized there's no "good" way for non JSX users to override autocomplete.render without importing Preact themselves.

If I'm not mistaken, what you're suggesting would be a breaking change of how render currently works, correct?

@Haroenv
Copy link
Contributor

Haroenv commented Jan 26, 2022

Not necessarily, the function response right now is used for nothing, so if you don't call render, nothing happens, meaning we can use the jsx as children if they're returned instead of rendered

@sarahdayan
Copy link
Member Author

Okay, so you'd imagine "transparent" rendering where, if user return elements, we render them internally. Something like:

autocomplete({
  // ...
  render({ sections }) {
    return (
      <div className="aa-PanelLayout aa-Panel--scrollable">{sections}</div>
    );
  },
});

Right?

@sarahdayan
Copy link
Member Author

Following up—this change might be a little premature indeed, as we may want to change the way we render.

Your suggestion is interesting @Haroenv, and if we do that we probably want to introduce a different option for it and deprecate render. Possibly related to the Layout API?

For now, if we decide on tabling this until we work on templating, users will still need to pull a VDOM implementation if they specify render. Are we cool with it?

@Haroenv
Copy link
Contributor

Haroenv commented Jan 27, 2022

as long as they use the same preact version, it's not duplicated at least?

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.

2 participants