Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

fix(highlight): change expected tag to replace #679

Merged
merged 6 commits into from
Jun 3, 2019
Merged

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented May 31, 2019

fixes #639

to fully fix #639, we also want algolia/instantsearch#3830 to be merged, however those are completely separate issues

@Haroenv Haroenv requested a review from a team May 31, 2019 12:19
@ghost ghost requested review from eunjae-lee and tkrugg and removed request for a team May 31, 2019 12:19
@Haroenv Haroenv requested review from francoischalifour and removed request for tkrugg May 31, 2019 12:20
src/components/Highlight.vue Outdated Show resolved Hide resolved
src/components/Highlight.vue Outdated Show resolved Hide resolved
src/components/Highlight.vue Outdated Show resolved Hide resolved
src/components/Highlight.vue Outdated Show resolved Hide resolved
@Haroenv
Copy link
Contributor Author

Haroenv commented May 31, 2019

I checked it, and this change also fixes it for searchable

@francoischalifour
Copy link
Member

The stories don't seem to work, they display <mark>Text</mark>.

@Haroenv
Copy link
Contributor Author

Haroenv commented Jun 1, 2019

That’s because of InstantSearch’s PR not yet been used here (there’s two hits widgets). You can check a story with only one hits to see it working compared to previously.

@Haroenv Haroenv merged commit 4aa06fb into master Jun 3, 2019
@Haroenv Haroenv deleted the fix/highlight-tag branch June 3, 2019 09:53
Copy link
Contributor

@samouss samouss left a comment

Choose a reason for hiding this comment

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

Now that InstantSearch 3.x.x is available we should be able to use highlight & snippet that are available on the default export. We don't have to duplicate the logic to perform this operation.

innerHTML() {
  return instantsearch.highlight({
    attribute: this.attribute,
    hit: this.hit,
    highlightedTagName: this.highlightedTagName,
  });
},

@Haroenv
Copy link
Contributor Author

Haroenv commented Jun 4, 2019

That’ll only work with escapeHTML: false, which is why I didn’t go for it

@samouss
Copy link
Contributor

samouss commented Jun 4, 2019

You're sure? I've tested it without issues. We use the same strategy inside InstantSearch.js.

@Haroenv
Copy link
Contributor Author

Haroenv commented Jun 4, 2019

It's possible that it works, but in the end it would be best if we migrate the InstantSearch highlight strategy to the one of React InstantSearch where there's no need for escaping or innerHTML, but you can map over the parts

@benjam-es
Copy link

highlighted-tag-name still does not work for me. Is the above fix tagged?

I'm on "vue-instantsearch": "^2.2.1"

@francoischalifour
Copy link
Member

@benjam-es The issue is fully fixed since #691 which updates the InstantSearch.js dependency. Vue InstantSearch wasn't released since. The highlighted-tag-name behavior will work as expected in the next version.

Haroenv added a commit to algolia/instantsearch that referenced this pull request Dec 28, 2022
…arch#679)

* fix(highlight): change expected tag to replace

* chore: remove comment

* Apply suggestions from code review

* fix: update tests

* chore: inline variables
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v2 – Highlight component ignores some props
4 participants