fix(preset-algolia): keep separator highlight when reverse-highlight siblings disagree#1348
Merged
Haroenv merged 1 commit intoJun 19, 2026
Conversation
…siblings disagree isPartHighlighted defaulted neighbor highlight states with `|| true`, which also overrode an existing `false` neighbor. That made both siblings always appear highlighted, so every non-alphanumeric separator was treated as highlighted-from-siblings regardless of its real neighbors. Use `?? true` so the default only applies when a neighbor is missing.
Up to standards ✅🟢 Issues
|
devteamaegis
force-pushed
the
fix/reverse-highlight-sibling-state
branch
from
7e6df5f to
3837461
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect separator highlighting in the autocomplete-preset-algolia reverse-highlight / reverse-snippet “sibling strategy” by ensuring sibling highlight state defaults only when a sibling is missing (not when it exists and is false).
Changes:
- Replace
|| truewith?? trueinisPartHighlightedso realfalsesibling states are preserved. - Add a regression test covering the “siblings disagree” case to prevent separator parts from incorrectly inheriting sibling highlight state.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/autocomplete-preset-algolia/src/highlight/isPartHighlighted.ts | Fixes sibling highlight defaulting logic by using nullish coalescing (??) instead of logical OR (` |
| packages/autocomplete-preset-algolia/src/highlight/tests/isPartHighlighted.test.ts | Adds a regression test asserting separators keep their own highlight state when adjacent parts disagree. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Haroenv
approved these changes
Jun 19, 2026
Haroenv
left a comment
Haroenv
left a comment
Contributor
There was a problem hiding this comment.
thanks, this is correct!
Haroenv
added a commit
to algolia/instantsearch
that referenced
this pull request
Jun 26, 2026
…disagree (#7079) * fix(highlight): keep separator state when reverse-highlight siblings disagree `getHighlightFromSiblings` defaulted neighbor highlight state with `|| true`: const isNextHighlighted = parts[i + 1]?.isHighlighted || true; const isPreviousHighlighted = parts[i - 1]?.isHighlighted || true; `X || true` is always `true`, even when the neighbor exists and is genuinely `false`. So the `isPreviousHighlighted === isNextHighlighted` guard is always satisfied and every separator part is treated as inheriting its siblings' highlight state — even when the siblings disagree. For a value where a separator sits between a highlighted and a non-highlighted segment, the reverse-highlight / reverse-snippet output is wrong. The `?.` optional chaining shows the intent was "default only when the neighbor is missing". Use `??` so the `true` default applies only when the neighbor is absent (`undefined`), preserving a real `false`. Mirrors algolia/autocomplete#1348, which fixes the same bug in the `autocomplete-preset-algolia` port of this sibling strategy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(highlight): update reverseHighlightedParts expectation for fixed sibling logic The "multiple matches" case asserted the buggy output: a separator between two non-highlighted parts ('Fi' and 'Black') was reversed to non-highlighted, leaving a gap in an otherwise contiguous reverse-highlighted run. With the `?? true` fix the separator correctly inherits its (agreeing) siblings, so the reversed "Fi - Black" run is now uniformly highlighted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's broken
In the reverse-highlight / reverse-snippet "sibling strategy",
isPartHighlightedcomputes neighbor state with||:X || trueis alwaystrue, even when the neighbor exists and is genuinelyfalse. So theisPreviousHighlighted === isNextHighlightedguard is always true, and every separator part between mismatched neighbors is wrongly treated as highlighted-from-siblings. For a value where a separator sits between a highlighted and a non-highlighted segment, the reverse-highlight output is wrong.Why it happens
||was used where a "default only when the neighbor is missing" was intended — the?.optional chaining shows the intent.false || truecollapses a realfalseneighbor totrue.Fix
Use
??so thetruedefault applies only when the neighbor is absent (undefined).Test
Added a case: a separator between a highlighted and a non-highlighted segment keeps its own state; fails before, passes after. Full
autocomplete-preset-algoliasuite stays green.