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(highlight): add cssClasses to snippet & highlight helper #4306

Merged
merged 5 commits into from
Feb 19, 2020

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Jan 21, 2020

This is related to: algolia/vue-instantsearch#759. Just adding this doesn't totally solve the situation yet, but it at least allows a hook for adding extra class names.

What's interesting to note is that the suit helper in InstantSearch only concatenates names, and thus the keys for cssClasses are just the modifier. In Vue InstantSearch the keys for classNames are the generated classes, since the suit helper will check those keys directly. I wonder if we could move to such a system in InstantSearch.js in the future. This will allow us to remove these mappings, and deal with it at call site (which likely has a decrease in bundle size)

Summary

Before this change, it was impossible to override class names for the highlighted part of an ais-highlight.

Result

A new cssClasses option to the highlight helper, which accepts a key highlighted.

This is related to: algolia/vue-instantsearch#759. Just adding this doesn't totally solve the situation yet, but it at least allows a hook for adding extra class names.

What's interesting to note is that the suit helper in InstantSearch only concatenates names, and thus the keys for cssClasses are just the modifier. In Vue InstantSearch the keys for classNames are the generated classes, since the suit helper will check those keys directly. I wonder if we could move to such a system in InstantSearch.js in the future. This will allow us to remove these mappings, and deal with it at call site (which likely has a decrease in bundle size)
@Haroenv Haroenv requested review from francoischalifour and a team January 21, 2020 09:54
@ghost ghost requested review from eunjae-lee and removed request for a team January 21, 2020 09:54
Copy link
Member

@francoischalifour francoischalifour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to increase bundlesize's threshold.

src/helpers/highlight.ts Show resolved Hide resolved
src/helpers/snippet.ts Show resolved Hide resolved
Copy link
Contributor

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me (as long as you add a commit to increase bundlesize's threshold)

Copy link
Member

@francoischalifour francoischalifour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good once the size threshold is increased.

@Haroenv Haroenv merged commit ece0aa6 into master Feb 19, 2020
@Haroenv Haroenv deleted the feat/hl-css-classes branch February 19, 2020 08:46
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.

3 participants