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

[next] Fixes #1484: Re-Adds the Search page component #1621

Merged
merged 3 commits into from
Feb 5, 2021

Conversation

chrispinkney
Copy link
Contributor

@chrispinkney chrispinkney commented Jan 29, 2021

Issue This PR Addresses

Fixes #1484

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI
  • Next Port: Port to NextJS

Description

This PR adds search.tsx, a page which will render the <SearchPage /> component. It also injects SEO along with the banner and head components.

EDIT: The <SearchPage /> component was just land-ho'd, so I merged master into this PR, renamed the search.tsx component @c3ho to SearchPage, added SEO to our search page, the banner, head, and darkmode toggle button.

Just waiting on <SearchPage /> to be finished then I can test this further.

CI/CD will fail as the <SearchPage /> component isn't yet implemented, when it gets released I'll (try to remember to) come back and test this further.

How to test

Pull the PR and navigate to http://localhost:8000/search.

firefox_2021-02-04_15-48-23

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@chrispinkney chrispinkney added this to the 1.6 Release milestone Jan 29, 2021
@chrispinkney chrispinkney self-assigned this Jan 29, 2021
@chrispinkney chrispinkney changed the title Added placeholder search component [next] Fixes #1484: Re-Adds the Search page component Jan 29, 2021
src/frontend/next/src/pages/search.tsx Show resolved Hide resolved
c3ho
c3ho previously approved these changes Feb 4, 2021
Copy link
Contributor

@c3ho c3ho left a comment

Choose a reason for hiding this comment

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

I'm fine with this, since it just moves some stuff around

@chrispinkney
Copy link
Contributor Author

Looks like Darkmode will need to be fixed for Searchpage. When this lands we'll have to open another issue for that.

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

One minor thing and this is good. If you want to file an issue to fix that later (it's in other pages too), that's also fine.

<>
<Head>
<title>Search</title>
<meta property="og:title" content="Telescope" key="title" />
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't dong anything there, let's remove and have it happen in _document.tsx

@chrispinkney chrispinkney merged commit 236a160 into Seneca-CDOT:master Feb 5, 2021
izhuravlev pushed a commit to izhuravlev/telescope that referenced this pull request Feb 5, 2021
…eca-CDOT#1621)

* Added Search component and moved search.tsx contents to SearchPage component
* Added Head, Darkmode Toggle, and Banner
@chrispinkney chrispinkney deleted the issue-1484 branch April 22, 2021 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: front-end area: nextjs Nextjs related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[next] Porting Search page component, see src/frontend/src/pages/search.js from Gatsby to Next
4 participants