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

fix(search-algolia): pass custom transformItems function to SearchBar #8462

Merged
merged 2 commits into from Dec 29, 2022

Conversation

mturoci
Copy link
Contributor

@mturoci mturoci commented Dec 19, 2022

Pre-flight checklist

Motivation

Closes #8462

Test Plan

Manual

Test links

Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/

Related issues/PRs

Closes #8461

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 19, 2022
@netlify
Copy link

netlify bot commented Dec 19, 2022

[V2]

Name Link
🔨 Latest commit a3e4b1e
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/63ad82b29d0f6b000894ba73
😎 Deploy Preview https://deploy-preview-8462--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Dec 19, 2022

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 78 🟢 97 🟢 100 🟢 100 🟢 90 Report
/docs/installation 🟠 74 🟢 100 🟢 100 🟢 100 🟢 90 Report

@Josh-Cena Josh-Cena changed the title feat: Allow passing custom transformItems function to Searchbar component. #8461 feat(search-algolia): pass custom transformItems function to SearchBar Dec 21, 2022
@Josh-Cena Josh-Cena added the pr: new feature This PR adds a new API or behavior. label Dec 21, 2022
Comment on lines 174 to 190
props.transformItems
? props.transformItems
: (items) =>
items.map((item) => {
// If Algolia contains a external domain, we should navigate without
// relative URL
if (isRegexpStringMatch(externalUrlRegex, item.url)) {
return item;
}

// We transform the absolute URL into a relative URL.
const url = new URL(item.url);
return {
...item,
url: withBaseUrl(`${url.pathname}${url.hash}`),
};
}),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Extract the arrow to a new variable and write useRef(props.transformItems ?? defaultTransformItems).

@slorber
Copy link
Collaborator

slorber commented Dec 22, 2022

This PR references itself (instead of an external issue) and does not really give much context into what you are trying to achieve (although I have an idea, I'd like you to explain)


This is also doing to conflict with #8428, which may already solve your use-case?


Note: make sure your use-case also works on the SearchPage? https://docusaurus.io/search

@slorber
Copy link
Collaborator

slorber commented Dec 22, 2022

Found the original issue 🤪 #8461

Using a wrapper to provide a custom transformItems prop is exactly what I had in mind, so feel free to go ahead.

Still wonder if the newly introduced code does not solve your use-case? cf #8428

  const transformItems = useRef<DocSearchModalProps['transformItems']>(
    (items) =>
      items.map((item) => ({
        ...item,
        url: processSearchResultUrl(item.url),
      })),
  ).current;

Why do you want to transform those items?

# Conflicts:
#	packages/docusaurus-theme-search-algolia/src/theme/SearchBar/index.tsx
@slorber slorber added the to backport This PR is planned to be backported to a stable version of Docusaurus label Dec 29, 2022
@slorber slorber changed the title feat(search-algolia): pass custom transformItems function to SearchBar fix(search-algolia): pass custom transformItems function to SearchBar Dec 29, 2022
@slorber slorber added pr: new feature This PR adds a new API or behavior. pr: bug fix This PR fixes a bug in a past release. and removed pr: new feature This PR adds a new API or behavior. labels Dec 29, 2022
@slorber slorber merged commit 0985fa0 into facebook:main Dec 29, 2022
@mturoci
Copy link
Contributor Author

mturoci commented Jan 9, 2023

Sorry @slorber @Josh-Cena. I was on a Christmas holiday but seems like you guys took care of this PR already.

This PR references itself

Oops, my bad.

Still wonder if the newly introduced code does not solve your use-case

Not really. I wanted to tweak the search results and reorder them the way I want. The linked issue allows only changing the results pathname if I could understand correctly.

@mturoci mturoci deleted the feat/issue-8461 branch January 9, 2023 07:19
slorber pushed a commit that referenced this pull request Jan 26, 2023
…#8462)

Co-authored-by: sebastienlorber <lorber.sebastien@gmail.com>
closes undefined
Closes #8462
Closes #8461
@slorber slorber added backported This PR has been backported to a stable version of Docusaurus and removed to backport This PR is planned to be backported to a stable version of Docusaurus labels Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR has been backported to a stable version of Docusaurus CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass transformItems to SearchBar
4 participants