ammachado commented on PR #1729: URL: https://github.com/apache/camel-website/pull/1729#issuecomment-5300650359
## Code review follow-up (d84cef15) A review pass over the DocSearch v5 migration turned up six findings; all are addressed in `d84cef15`. Summarizing the two that changed behavior, since they are worth a look rather than just a rubber stamp. ### Search result curation was silently dropped The deleted `src/js/vendor/algoliasearch.bundle.js` filtered sub-project hits, de-duplicated hits pointing at the same parent page, and ranked core docs above component pages. The v5 config carried none of that over. Restored in adapted form: - **Sub-project exclusion** via `transformItems`. - **Core-docs ranking** via `transformItems`. This works because DocSearch groups by `hierarchy.lvl0` into an insertion-ordered plain object (`buildQuerySources` in `@docsearch/react`), so the order we emit decides both group order and within-group order. - **Per-page cap** of 2 hits, replacing the old query-aware `deduplicateHits`. `transformItems` never receives the query, and the original only used it to detect a direct parent match, so capping by page reaches an equivalent outcome without it. - **`hitsPerPage: 50`** instead of the DocSearch default of 20. That last one is a real regression fix, not tuning. Both filters run client side, so they operate on an already-truncated window. Distinct parent pages surviving the sub-project filter, measured against the live index: | query | hitsPerPage 20 | hitsPerPage 50 | |---|---|---| | kamelet | 2 pages | 3 pages | | timer | **1 page** | 7 pages | | rest dsl | 7 pages | 16 pages | At the default, `timer` returned 19 usable hits spanning a single document, so the first result group was five near-identical anchors off one page. The durable fix is at the index level rather than in the browser: the index has no `attributeForDistinct` and no `attributesForFaceting`, so none of this can move server side from here. Filed as **[CAMEL-24396](https://issues.apache.org/jira/browse/CAMEL-24396)**. ### `@docsearch/css` lost its runtime cascade The vendor stylesheet was going through `postcss-custom-properties` with `preserve: false`, which flattened it. Verified in the built output: the `@media (width<=768px)` `:root` overrides were inlined at desktop values, `--shimmer-bg` was dead, and the 2.5 KB `:root[data-theme=dark]` block still shipped but could never take effect. `@docsearch/css` now gets its own build stream that skips the custom-property pass, revved to `css/vendor/docsearch.css` and linked ahead of `site.css` in both the Antora and Hugo head partials. This mirrors how the DocSearch UMD bundle already ships as vendor JS. I rejected the one-line `preserve: true` alternative deliberately: it fixes the same bug but costs about 3.8 KB gzip site-wide and would reverse a flattening convention that predates this branch. Total CSS actually shrank slightly, since DocSearch's properties are no longer duplicated by inlining. ### Also fixed - The new `drop-resolved-custom-properties` plugin was deleting `@media`-nested `:root` definitions that `postcss-custom-properties` never substitutes (exactly the block `@docsearch/css` ships), while missing resolved `:root, :host` list forms. - The `@docsearch/js` UMD path now fails loudly instead of surfacing as an opaque vinyl-fs glob error. - Added a `window.docsearch` guard so a vendor-bundle load failure cannot take down the rest of the `site.js` listener chain. ### Tests 16 cases, all passing. The search-transform suite evaluates the shipped IIFE against stubbed globals and replays captured Algolia responses, so it covers the real file rather than a copy. Mutation-checked: weakening the per-page cap, dropping to `hitsPerPage: 20`, or removing the sort each fails the relevant assertions. ### Still to verify Nothing has been exercised in a browser yet. Keeping this as a draft until the DocSearch modal on mobile, the `data-theme="dark"` toggle, and the `timer` / `kamelet` result lists are checked against a running site. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
