Closed mthuret closed 1 year ago
That's some massive research and API proposal, GG!
You can already use Style Component with React InstantSearch but it requires the need of wrapper.
Do we already have an example of that? Mostly curious
Glamor/Glamorous
It can be used by injecting global css, they have an API for that
Any small example?
I'm not sure the Styled Component modification should be done yet.
Provide an example so we can better understand the struggle versus your proposal, thanks!
I'm also thinking that maybe the react-themeable approach we used at first was not that bad and we should considering putting it back? WDYT?
This looks like a good solution. We got away of react themeable mostly because it was complex to understand everything it was doing, we thought we might just remove it and add something else later (which is now).
If we address via custom APIS and guides:
Then we will be in a good state already.
Seems that material ui recently revamped their styling customization: https://material-ui-1dab0.firebaseapp.com/customization/overrides/, @oliviertassinari any comments to help us here from you is welcomed. Your experience and user feedback from material ui is appreciated.
@vvo Thanks for the introduction, I can speak for the tradeoffs we have chosen for Material-UI.
we choose to only give the ability to override their style using our BEM classnames
We have explicitly removed this option in order to be able to optimize our class names length in production. But we lose this no brainer override option.
The most interesting part from Material-UI would be the classes
property section of the documentation. Let's take the Button
as an example. The component has some nested elements as well as some internal state. Providing a className
property is not enough. With the classes
property users can inject their own style. The list is automatically generated here
We also have an even more powerful override mechanism that relies on the context to change the style even before it's injected into the DOM. But it's probably not relevant here.
About a better integration of css frameworks: after discussing this with @vvo we came to the conclusion that using a css framework shouldn't be difficult. It is now because we compute a lot of logic directly in our widgets (Like the Pagination one) where we could ultimately compute those under the connector.
Instead of providing a mapping classnames API what we are proposing to do is:
I started to play around with adding 3 levels of customization to the class name generator:
import { setPrefix } from 'react-instantsearch/components/classNames';
setPrefix('foo');
// now ais-HierarchicalMenu__root is foo-HierarchicalMenu__root
import { setClassMapper } from 'react-instantsearch/components/classNames';
const myMap = {
'ais-HierarchicalMenu__root': 'my-menu-root instantsearch-component'
};
setClassMapper(className => myMap[className] || className);
import cx from 'classnames';
import { setFormatter } from 'react-instantsearch/components/classNames';
setFormatter((block, elements) => ({
className: cx(
elements
.filter(element => element !== undefined && element !== false)
.map(element => `myapp-${block}-${element}`)
.map(classMap)
),
});
A basic implementation: https://gist.github.com/inxilpro/1849451f3479edb097530e342c1cd371
This is a pretty easy fix that adds a ton of customization options without too much rethinking of the library.
If, for example, you wanted to use tailwind-css, you could do something like:
import cx from 'classnames';
import { setFormatter } from 'react-instantsearch/components/classNames';
const map = {
'HierarchicalMenu': {
'root': 'flex flex-1',
'itemLink': 'text-blue uppercase no-underline',
'itemCount': 'float-right text-white rounded bg-grey p-2',
}
};
setFormatter((block, elements) => ({
className: cx(
elements
.filter(element => element !== undefined && element !== false)
.map(element => map[block] && map[block][element]
? map[block][element]
: `${block}-${element}`
)
),
});
Hey!
We're doing a round of clean up before migrating this repository to the new InstantSearch monorepo. This issue seems not to have generated much activity lately and to be mostly solved, so we're going to close it.
In React InstantSearch Hooks, all widgets accept props of their wrapping elements, so you can easily use them with libraries like Styled Components. Here's an example that uses Styled Components and even set inline styles on the <StyledHits>
: https://codesandbox.io/s/intelligent-dust-0vze0s?file=/src/App.tsx
Otherwise, for usage with utility CSS like TailwindCSS, Bootstrap, Tachyons, etc., you can use the classNames
option on each widget.
When designing our widgets we choose to only give the ability to override their style using our BEM classnames.
This has limitations: you can't add your own classnames and use Bootstrap-like css framework. See https://github.com/algolia/instantsearch.js/issues/2089
We also have some limitations with the usage of CSS-In-JS librairies like describe here: https://github.com/algolia/instantsearch.js/issues/2089
Finally, if we decide to add some React Native widget, overriding inline style would also become an issue.
Here's a little summary of what's possible, what we could do depending of each way
Bootstrap-like framework:
Without the ability to add classnames, it requires the usage of connectors and therefore redoing everything. Also, this API will not fix everything as sometimes the markup will not correspond with the one of our widget.
=> the idea of the needed API is given a default class, returned the new one(s). I'm not sure this be at the InstantSearch level but at the widget level for a better clarity.
Could be like @vvo proposed a prop 'toCssClassName':
Styled component
You can already use Style Component with React InstantSearch but it requires the need of wrapper.
If we want to optimise this, then the prop
className
should be forwarded to every widget and apply on a root div that is not the one having 'ais-XXX__root' (otherwise style for this classname will not be applied). Not every widgets have this extra div, so it'll mean breaking changes.Glamor/Glamorous
It can be used by injecting global css, they have an API for that
Inline style
Same issue than for the class name override. It would require the same API but instead of returning a string, it would return an object and be applied as a
style
.Css module
It works by just importing a new css file.
If I miss something, please feel free to add it to the discussion.
In the end, It will require only one API entry. I'm not sure the Styled Component modification should be done yet.
I'm also thinking that maybe the react-themeable approach we used at first was not that bad and we should considering putting it back? You could write things like:
WDYT?