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

WIP: Add docs in search #841

Closed
wants to merge 2 commits into from
Closed

Conversation

gagdiez
Copy link
Collaborator

@gagdiez gagdiez commented Jul 8, 2024

This PR adds the ability to search for docs directly on the general searchbar

We might want to discuss:

  • Removing the "All" tab
  • Changing the "Apps" tab, so the result comes from NEAR Catalog instead of showing only components

@gagdiez gagdiez requested review from shelegdmitriy and charleslavon and removed request for shelegdmitriy July 8, 2024 14:38
@gagdiez gagdiez added the enhancement To highlight a PR's description in the 'Enhancements' changelog section label Jul 8, 2024
Copy link
Contributor

@shelegdmitriy shelegdmitriy left a comment

Choose a reason for hiding this comment

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

Overall looks good! Thank you @matiasbenary for contribution! But I want to highlight that all this changes should be added to src/Search/IndexPage.jsx page also.

Removing the "All" tab

Well, I think we can remove this tab from TypeAheadDropdown but let's leave it on src/Search/IndexPage.jsx. There is enough space for that there. WDYT?

Changing the "Apps" tab, so the result comes from NEAR Catalog instead of showing only components

Apps tab already has the nearcatalog records if I'm not mistaken 🤔

Also, I do understand that this PR is in progress but I think this UI could be improved a bit:
Знімок екрана 2024-07-10 о 17 22 11

  1. Let's display a doc's category instead of COMPONENTS 2389 (or Docs after changes). Seems like we do have such data so why don't we use it?
  2. Let's add a hover effect like text-decoration: underline for title or something

Comment on lines +736 to +748
case "Docs":
return state.components.hits?.length > 0 ? (
<DisplayResultsByFacet title="Components" count={state.docs.hitsTotal} items={topmostComponents(false)} />
) : (
<>
<TextMessage message={`No Component matches were found for "${state.term}".`} />
<TextMessage
message={`Trying to find a app built by an user? Try search their account id.`}
$fontSize="12px"
$top="45%"
/>{" "}
</>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
case "Docs":
return state.components.hits?.length > 0 ? (
<DisplayResultsByFacet title="Components" count={state.docs.hitsTotal} items={topmostComponents(false)} />
) : (
<>
<TextMessage message={`No Component matches were found for "${state.term}".`} />
<TextMessage
message={`Trying to find a app built by an user? Try search their account id.`}
$fontSize="12px"
$top="45%"
/>{" "}
</>
);
case "Docs":
return state.docs.hits?.length > 0 ? (
<DisplayResultsByFacet title="Docs" count={state.docs.hitsTotal} items={state.docs.hits} />
) : (
<>
<TextMessage message={`No matches were found for "${state.term}".`} />
</>
);

@@ -562,6 +669,9 @@ const tabCount = (tab) => {
case "Apps":
// Return the count for Apps
return state.apps.hitsTotal ?? 0;
case "Docs":
// Return the count for Apps
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Return the count for Apps
// Return the count for Docs

Comment on lines +685 to +687
if (state.selectedTab === "Docs") {
return <Items>{state.docs.hits}</Items>;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (state.selectedTab === "Docs") {
return <Items>{state.docs.hits}</Items>;
}

Let's remove this changes and apply my next suggestion instead

@shelegdmitriy
Copy link
Contributor

Hey @matiasbenary and @gagdiez! Seems like there is no need in this PR in favour of this near/near-discovery#1249 right?

@gagdiez
Copy link
Collaborator Author

gagdiez commented Jul 18, 2024

@shelegdmitriy indeed, closing this one

@gagdiez gagdiez closed this Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement To highlight a PR's description in the 'Enhancements' changelog section
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants