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

DOP-3785: adding a persistent search bar #843

Merged
merged 12 commits into from
Jul 11, 2023
Merged

DOP-3785: adding a persistent search bar #843

merged 12 commits into from
Jul 11, 2023

Conversation

bianca-laube
Copy link
Contributor

Stories/Links:

DOP-3785

Current Behavior:

https://www.mongodb.com/docs/search/?q=test
Screenshot 2023-06-21 at 4 42 22 PM

Staging Links:

https://docs-mongodb-org-stg.s3.us-east-2.amazonaws.com/master/landing/bianca.laube/DOP-3785/search/index.html?q=test
Screenshot 2023-06-21 at 4 49 14 PM

Notes:

Things Changed:

  • Updated header
  • The staging link goes automatically to the search page since if you try to go the search page through the home page it leaves the staging link. However, the first word given is taken from that search bar, from the header and automatically loaded to the new persistent search bar.
  • was able to add the new leafygreen-ui search-input for the search bar

@bianca-laube bianca-laube marked this pull request as draft June 23, 2023 14:58
@bianca-laube bianca-laube marked this pull request as ready for review June 23, 2023 15:38
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

looks great!! thanks for putting this together @bianca-laube

a couple comments on persisting the search bar on empty results. will need to consult with Product team to verify base case scenarios

src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

some minor differences in behavior from whats on prod vs this branch (with flag set to "false") PTAL !

src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

some minor styling nitpicks to keep prod untouched. also not seeing loading states with the new component state variables. PTAL! 🙏

src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
@bianca-laube bianca-laube requested a review from mmeigs July 6, 2023 17:58
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

this overall LGTM. one minor functional catch below (onSubmit function for SearchInput)

thanks again for putting this together @bianca-laube !

src/components/SearchResults/SearchResults.js Show resolved Hide resolved
Copy link
Collaborator

@mmeigs mmeigs left a comment

Choose a reason for hiding this comment

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

Looks great! I believe there are a couple unused conditionals! Check me on it though, because there are so many layers to unpack.

Also, I feel like I may have found a bug with our search, but it's not a part of this ticket. Any one letter search seems to hang in loading state oddly. Something to keep an eye on. Bonus points!

src/components/SearchResults/SearchResults.js Show resolved Hide resolved
src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
src/components/SearchResults/SearchResults.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

this looks great! minor styling issue below and a possible refactor (not blocking). besides that LGTM !

@bianca-laube bianca-laube merged commit 440d22b into master Jul 11, 2023
2 checks passed
@bianca-laube bianca-laube deleted the DOP-3785 branch July 11, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants