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

Implement search UI #25

Merged
merged 27 commits into from
Sep 14, 2021
Merged

Implement search UI #25

merged 27 commits into from
Sep 14, 2021

Conversation

jonathansick
Copy link
Contributor

@jonathansick jonathansick commented Aug 2, 2021

This PR implements a search UI with Algolia instantsearch.

This work is built around the concept that content and search results should be shown directly on the homepage rather than in per-category listing pages. To do this I've pared down the UI so we don't have the category cards and large search box on the homepage.

This replaces #22 which was built around the idea of per-category result pages.

A demo of the design is available at https://astropy-search-demo.jsick.dev/

@jonathansick jonathansick mentioned this pull request Aug 2, 2021
@jonathansick
Copy link
Contributor Author

This should be merged after #24

@jonathansick jonathansick marked this pull request as ready for review September 2, 2021 16:29
@jonathansick jonathansick requested a review from adrn September 2, 2021 16:29
@jonathansick
Copy link
Contributor Author

@adrn There's still more to do but this is 80% of it, and it might be easier to make smaller improvements in individual PRs after this.

@adrn
Copy link
Member

adrn commented Sep 3, 2021

Awesome!! I'll take a look over the next few days.

Copy link
Member

@adrn adrn left a comment

Choose a reason for hiding this comment

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

Only a few minor comments! I didn't review the CSS closely but the styling in your demo site looks good to me :)

<p>
The Learn Astropy website was developed under a grant by the{' '}
<a href="http://www.dunlap.utoronto.ca">
Universitory of Toronto Dunlap Institute for Astrophysics.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Universitory of Toronto Dunlap Institute for Astrophysics.
University of Toronto Dunlap Institute for Astrophysics.

<img
className="numfocusStamp__image"
src={numfocusStamp}
alt="A fiscally sponsored project of Numfocus"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
alt="A fiscally sponsored project of Numfocus"
alt="A fiscally sponsored project of NumFOCUS"

Comment on lines +5 to +6
'KSDFJKHCO2',
'fadf7c74af324735d9e45d68481531ab'
Copy link
Member

Choose a reason for hiding this comment

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

Is it ok for this to be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, they're meant to be on the client-side and are read-only credentials.

@eblur
Copy link

eblur commented Sep 7, 2021

This looks good to me! I like that each card has a "Guide", "Tutorial", etc attached to it. I'm glad that they are labeled in that way.

One issue I note is that there is probably a lot of metadata still missing in order to properly search and sort. For example, the CCD Data Reduction Guide relies heavily on ccdproc, a package that is being developed to replace IRAF and will be of key interest to many people.

@eblur
Copy link

eblur commented Sep 7, 2021

I would not require my above note to be fixed to accept the PR. After some discussion, we noticed it is related to astropy-librarian Issue 14

With the move to put the search UI on the homepage and combine results
from all content categories, we no longer want the large header with
hero search and the content category cards on the homepage. Instead, we
want that header to shrink into something more like a thin global nav
header that contains the site logo and links.
Add more design tokens for defining a range of sizes and neutral colors,
and map those tokens to specific use-cases.

Overall, this helps maintains consistency in the design so that sizes
and colours are more consistent across the site.

In the future we'll want to build out a full grid of neutral and primary
colors to use in different situations.
This component works well with the new NavHeader. It presents a
full-width background and is meant to encompass the page's title and a
lead paragraph or graphic.
This will make space for adding the search UI.
Adds algoliasearch 4.10.3 and thr react-instantsearch-dom ui library
version 6.12.1.
This is loaded on the root layout.js for every page from the
instantsearch.css package.
This search client includes our Algolia project ID and read-only API
key. Both of these are safe and in fact intended to be used client-side.

Individual search UIs can import and use this client with the
InstantSearch React component.
Breakpoints can't be set through CSS custom properties. Instead, we set
them in JS and use string interpolation in styled components to share
reusable breakpoint dimensions.
This adds the beginnings of a search UI to the index page. Currently it
uses stock UI components from react-instantsearch.

The SearchLayout component implements a two-column search layout with
refinements on the left.
The ResultCard styles individual results. The StyledHits component and
other components in src/components/instantsearch/hits.js serves to
connect the custom UI into the instantsearch UI core.
Use flexbox to create a side-by-side grid that adapts if there isn't a
thumbnail image.

Fix side-by-side card appearance
Refine the listing based on keywords and content type. Note that all
these attributes must be configured as "facets" in Algolia.
And gatsby-plugin-styled-components to 4.12.0
It turns out that styled-components dropped support for the @Viewport
selector that was still in our boilerplate (now using a meta tag for
this anyways). But that was preventing many global styles on the :root
and html element from being parsed in the static site build.

Also moved the <GlobalStyles> outside the <StyledLayout> tag in the
Layout component because I think this is more correct (I haven't tested
whether this also contributed to the problem, in isolation, though).
This is required for free/community/open source usage of Algolia.
Eventually we may want to associate different icons or tags with
different content types; but for now this works great.
Show the page title if the hit is within the guide; otherwise the
guide's title is show with a link to the homepage.

Also, when showing the page title, also include the title of the guide
itself to privide context.
This component implements the feature where "priority" resources are
shown preferentially when a user doesn't enter a search term; but when a
search term is entered the sort automatically switches to "relevance" to
provide the best search match.

Behind the scenes, the PrioritySort component is switching between two
different Algolia indices. The relevance-based index is the default; the
priority index is a "replica" index that has a different sort order to
leverage the priority field in our Algolia records.
This creates the link column style of footer navigation.
This lets us add a background, much like the page cover and header.
@jonathansick
Copy link
Contributor Author

@eblur That's a really good point about the keyword metadata for guides. We'll need to either establish a metadata file to go along with a guide to provide that or start extracting Python package usage from code samples as discussed in astropy/learn-astropy-librarian#14 (which has some up-front challenges, but would be a nice system to have in the long run. Either way, the front-end wouldn't need to change to make that possible, so it's something I'll need to add to https://github.com/astropy/learn-astropy-librarian. I do think a metadata header standard or metadata file would be good for guides, and I've created a second ticket for that: astropy/learn-astropy-librarian#17

@jonathansick jonathansick merged commit b9f9aba into astropy:main Sep 14, 2021
@jonathansick jonathansick deleted the add-search branch September 14, 2021 15:02
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