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

RFCs: Use render props for components / Use custom components #355

Closed
divyanshu013 opened this issue May 2, 2018 · 2 comments
Closed

RFCs: Use render props for components / Use custom components #355

divyanshu013 opened this issue May 2, 2018 · 2 comments
Labels
idea 🤔 Discuss new ideas or feature requests wontfix

Comments

@divyanshu013
Copy link
Contributor

divyanshu013 commented May 2, 2018

Issue Type

Enhancement and rewrite

Platform

Web currently but can extend for native too

Problem

Currently the components' presentational and data layer are tightly coupled. This works for the components since we provide a lot of customization around styling however it can become tedious and unnecessary if we already have a presentational component which we simply want to connect to ReactiveSearch. ReactiveComponent can also be used for this usecase but at times I feel its an overkill for simple UI components like search input box, dropdowns, lists, toggle buttons, etc.

For example, if someone wishes to use the Input component from ant design for performing search (and showing suggestions) instead of the default components from DataSearch or CategorySearch, they would prefer using ReactiveComponent since editing this much styling is time consuming and adds to the maintenance. The caveat is, they will have to write all the logic which is already available in DataSearch or CategorySearch (like handling suggestions, highlight, ... and the query generation).

Proposal

Separate out the presentational logic from components and allow using custom components. By default we can use the presentational components from ReactiveSearch but the user can specify his own component instead via (maybe) render props.

Here's a sample of how the API can look for a List component (I've taken some inspiration from Downshift and react-router (v4):

import { Dropdown } from 'antd';
import { List } from '@appbaseio/reactivesearch';

<List
  dataField="xyz"
  react={...}
  ...
>
  {( state, hits or aggregations) => <Dropdown ... />}
</List>

One more advantage of this approach is we need only a single list component that manages the reactive logic. Its the users choice if he wishes to render the list as a dropdown (which we currently do via SingleDropdownList) or a simple list (currently via SingleList). Similarily a single Range component would be able to handle all the reactive logic for dropdown or list (and maybe single/multi range too). This should also make testing a bit simpler for us and the user.

Request For Comment(s)

I haven't fully thought through the API yet but I'll try to come up with a POC for a single component so we can observe the differences. I'm also interested in how the community is using ReactiveSearch with their own UI components. Please feel free to add your views and suggestions 🙂


Update

I think we should only provide the data layer in reactivesearch and move the presentational components to design kit. This solves a lot of problems and issues when someone wants different UI behavior than we provide in our presentational layer. For example, consider the date components where one would want to use ant design, react-dates or some other solution for their use case. Another good example is multi selection, someone might want to use ant design's multiselect or react-select.

@divyanshu013 divyanshu013 added the idea 🤔 Discuss new ideas or feature requests label May 2, 2018
@divyanshu013 divyanshu013 changed the title RFCs: Use render props for components RFCs: Use render props for components / Use custom components May 2, 2018
@metagrover
Copy link
Contributor

This looks great! Couple of thoughts:

  • We will also have to expose the state-reducers, action-handlers and all of the redux store props along with the state in the render props method.

  • Need to think of this implementation for Server Side Rendering.

@stale
Copy link

stale bot commented Aug 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Aug 6, 2019
@stale stale bot closed this as completed Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea 🤔 Discuss new ideas or feature requests wontfix
Projects
None yet
Development

No branches or pull requests

2 participants