Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

feat: 🎸 search #1677

Merged
merged 17 commits into from
Nov 4, 2021
Merged

feat: 🎸 search #1677

merged 17 commits into from
Nov 4, 2021

Conversation

benhalverson
Copy link
Member

@benhalverson benhalverson commented Aug 1, 2021

Initial implementation of a client side search.
It current only searches through the /learn material.
The search bar is only visible on /learn or /learn/* at the moment.

Things todo in future PR's

  • allow keyboard shortcuts to open and close the search dropdown
  • improved accessibility
  • make it a global site search instead of just the learn docs

co-author: @Mpaulo95

@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 1, 2021
@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 1, 2021
@github-actions
Copy link

github-actions bot commented Aug 1, 2021

Please find a preview at: https://staging.nodejs.dev/1677/

Copy link
Member

@manishprivet manishprivet left a comment

Choose a reason for hiding this comment

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

Functionality-wise, it looks good to me.

We'll have to deal with these console errors though
image

One other thing I noticed is that the primary js file in the production bundle increased from 27.7KB to 52.6 KB, maybe because for client-side search it must have been bundling the data about all the learn pages everywhere.

If that's the case, this size is likely to increase a lot more when we start indexing docs and blogs, and that might be a concern.

nodejs.dev

image

This PR

image

@benhalverson
Copy link
Member Author

If that's the case, this size is likely to increase a lot more when we start indexing docs and blogs, and that might be a concern.

Good point... I didn't look at the bundle sizes. I did see an option to lazy load in the docs for https://www.gatsbyjs.com/plugins/gatsby-plugin-local-search/ but I get a cross-origin react error when I try to use it.
🤷🏻‍♂️
I think the current implementation is ok for now.

@benhalverson benhalverson force-pushed the search-lunr branch 2 times, most recently from df1f89b to a5e9be1 Compare August 7, 2021 05:25
@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 7, 2021
@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 7, 2021
@github-actions
Copy link

github-actions bot commented Aug 7, 2021

Please find a preview at: https://staging.nodejs.dev/1677/

@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 9, 2021
@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 9, 2021
@github-actions
Copy link

github-actions bot commented Aug 9, 2021

Please find a preview at: https://staging.nodejs.dev/1677/

@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 9, 2021
@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 9, 2021
@github-actions
Copy link

github-actions bot commented Aug 9, 2021

Please find a preview at: https://staging.nodejs.dev/1677/

@benhalverson benhalverson linked an issue Aug 10, 2021 that may be closed by this pull request
7 tasks
@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 13, 2021
@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 13, 2021
@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 14, 2021
@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 14, 2021
@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 14, 2021
@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 14, 2021
@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 14, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2021

Codecov Report

Merging #1677 (2537fa9) into main (f85dbb3) will decrease coverage by 1.78%.
The diff coverage is 55.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1677      +/-   ##
==========================================
- Coverage   90.67%   88.88%   -1.79%     
==========================================
  Files          81       83       +2     
  Lines         879      927      +48     
  Branches      241      262      +21     
==========================================
+ Hits          797      824      +27     
- Misses         81      102      +21     
  Partials        1        1              
Impacted Files Coverage Δ
src/components/SearchBar/index.tsx 33.33% <33.33%> (ø)
src/components/SearchBar/SearchInput.tsx 52.77% <52.77%> (ø)
src/components/Header/index.tsx 95.23% <75.00%> (-4.77%) ⬇️
src/containers/Navigation/index.tsx 68.18% <100.00%> (-1.59%) ⬇️
src/hooks/useMediaQuery.tsx 100.00% <0.00%> (ø)
util-node/createDocPages.js 100.00% <0.00%> (ø)
src/util/scrollTo.tsx 35.13% <0.00%> (+1.80%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f85dbb3...2537fa9. Read the comment docs.

@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 14, 2021
@github-actions
Copy link

Please find a preview at: https://staging.nodejs.dev/1677/

@github-actions github-actions bot removed the create-preview Generate preview on staging.nodejs.dev label Oct 17, 2021
@benhalverson benhalverson removed the request for review from lancemccluskey October 17, 2021 05:06
@github-actions
Copy link

Please find a preview at: https://staging.nodejs.dev/1677/

@benhalverson benhalverson force-pushed the search-lunr branch 2 times, most recently from 19e9d6f to 19fb363 Compare November 3, 2021 05:27
Copy link
Contributor

@designMoreWeb designMoreWeb left a comment

Choose a reason for hiding this comment

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

LGTM

@benhalverson benhalverson merged commit b4ce29e into nodejs:main Nov 4, 2021
nschonni added a commit that referenced this pull request Nov 5, 2021
kevindavies8 added a commit to kevindavies8/nodejs.dev-for-full-stack-developer that referenced this pull request Aug 24, 2022
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.

Feature: Search box
7 participants