algolia / instantsearch

⚡️ Libraries for building performant and instant search and recommend experiences with Algolia. Compatible with JavaScript, TypeScript, React and Vue.
https://www.algolia.com/doc/guides/building-search-ui/what-is-instantsearch/js/
MIT License
3.7k stars 516 forks source link

Missing indication on how to style the searchForFacetValues searchbar #2052

Closed pixelastic closed 5 years ago

pixelastic commented 7 years ago

Do you want to request a feature or report a bug? Feature.

Feature: What is your use case for such a feature? I want to make it match the rest of the design of my search. Neither the searchForFacetValues.templates, templates nor cssClasses options seem to accept anything that could let me change the styling of the searchbar. At least, I couldn't find any mention of it in the doc.

The pre-generated class names also does not seem to follow the ais- notation used in the other widgets, which makes me think they are internal classes and shouldn't be relied upon.

Feature: What is your proposed API entry? The new option to add? What is the behavior? Having the class names (currently searchbox, sbx-sffv, sbx-sffv__wrapper, sbx-sffv__input, etc) follow the ais- naming used in the other widgets would be enough for my use-case. It would give me the signal that I can style them and they won't change in an upcoming release.

image image

What project are you opening an issue for?

What is the version you are using? Always use the latest one before opening a bug issue. 1.11.2

bobylito commented 7 years ago

What would be a good way to document the css of the widgets? I think that even though v2 is a great upgrade on the doc, I kinda think we failed on the CSS part. Do you have examples of libraries that document their css well? @ronanlevesque @pixelastic @LukyVj @JonasBa

ronanlevesque commented 7 years ago

My POV is that it's rather a consistency problem than a doc problem. That being said, I do like the approach taken React IS on its docs, seems clear enough to me (use a table to display the class name and its role inside the component).

I guess we need to make sure that all our class names follow the same naming conventions in order to make it easier for people to understand what can be modified and its exact role.

Good news though: it's exactly what I'm working on! :)

bobylito commented 7 years ago

Yeah I know @ronanlevesque :D That's true that we don't really need the documentation to be based on the actual markup... That could be worth exploring, how do you feel about the number of classes? Isn't that too much?

ronanlevesque commented 7 years ago

I'd rather have more classes than less if it's justified (will avoid using complex selectors, which could lead to specificity issues).

That being said, I'm trying to simplify the DOM wherever possible

Haroenv commented 7 years ago

see also https://github.com/algolia/react-instantsearch/issues/263 for a discussion around class names

pixelastic commented 7 years ago

I could live with an HTML snapshot of the component, with all the needed classes added on it (if they have explicit-enough names). Otherwise, a breakdown of each HTML elements, with the list of classes and their usage in a table could work.

Agreed that I'd rather have more classes than not enough. vue-instantsearch has a nice mechanism that let you add your own classes as well and that would be my preferred solution (allowing for overriding existing classes, but also providing better integration with CSS frameworks)

bobylito commented 7 years ago

I could live with an HTML snapshot of the component, with all the needed classes added on it (if they have explicit-enough names). Otherwise, a breakdown of each HTML elements, with the list of classes and their usage in a table could work.

Was the v1 doc better on this aspect? The "view HTML" tab with the hover preview for the classes? https://community.algolia.com/instantsearch.js/v1/documentation/#starrating

ronanlevesque commented 7 years ago

Was the v1 doc better on this aspect? The "view HTML" tab with the hover preview for the classes? https://community.algolia.com/instantsearch.js/v1/documentation/#starrating

FYI InstantSearch.css will bring back this kind of option :)

bobylito commented 7 years ago

FYI InstantSearch.css will bring back this kind of option :)

That's good to know, but will it be convenient from the user PoV to go back and forth between two websites to check the markup? Maybe it could be good enough? Or maybe we could reuse part of the doc of instantsearch.css for that purpose?

ronanlevesque commented 6 years ago

I'd say it would be good enough. Since implementing widgets and styling them can be handled completely separately, I see no issue with having it this way.

spacecat commented 5 years ago

What is the status on this one? All HTML elements should be customizable imo. Otherwise what's the point of having a website design?

Does anyone have a good template/solution for this one?

Also, I'm wondering how you guys are dealing with the default SVG magnifying glass icon? I really don't like it, it doesn't fit with my design guidelines and I would like to change it to some other icon.

I feel that this particular searcbox should be designable exacly like the main searchbox. Also, it should have a nice default (like the main searchbox). I feel someone just took whatever was available at the time in terms of design/css and implemented that into the refinemenList.

Please give us a nice-looking default template with css classes for all elements + an easy way to change SVG icon.

Haroenv commented 5 years ago

I agree, this should be customisable, @francoischalifour: we can do these changes in v3

francoischalifour commented 5 years ago

Definitely! We'll work on that for InstantSearch.js 3.