Port Spinner from Gatsby to Next.js(TS) #1462

c3ho · 2020-11-26T03:59:47Z

Issue This PR Addresses

Fixes #1479

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

Description

Porting from Gatsby to Next JS + changing JS to TS for the Spinner component.

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)

manekenpix · 2020-11-26T05:24:17Z

src/frontend/next/src/components/SearchPage/SearchPage.tsx

@@ -0,0 +1,54 @@
+import React, { FC, FormEvent, useState } from 'react';


In this thread:

Next.js automatically adds the React import when JSX is used indeed. However keep in mind that we do still need to import React from 'react' when the React variable is used.

manekenpix · 2020-11-26T05:38:19Z

src/frontend/next/src/components/SearchPage/SearchResults.tsx

+type SearchPageProps = {
+  text: string;
+  filter: string;
+};


Suggested change

type SearchPageProps = {

text: string;

filter: string;

};

interface SearchPageProps {

text: string | null;

filter: string | null;

};

rogercyyu · 2020-11-30T16:48:23Z

whoops, sorry. Accidentally clicked that button.

humphd · 2021-01-15T20:00:59Z

@c3ho could you rebase this for package.json changes?

chrispinkney · 2021-01-20T03:24:02Z

src/frontend/next/package.json

-    "react-dom": "^16.13.1"
+    "react-dom": "^16.13.1",
+    "swr": "^0.3.9",
+    "use-query-params": "^1.1.9"


All of these are already merged into next now

chrispinkney · 2021-01-20T03:28:51Z

src/frontend/next/src/components/Spinner/Spinner.tsx

+import { FC } from 'react';
+import { makeStyles } from '@material-ui/core/styles';
+import CircularProgress from '@material-ui/core/CircularProgress';
+
+const useStyles = makeStyles((theme) => ({
+  root: {
+    display: 'flex',
+    '& > * + *': {
+      marginLeft: theme.spacing(2),
+    },
+  },
+}));
+
+const Spinner: FC = () => {


Suggested change

import { FC } from 'react';

import { makeStyles } from '@material-ui/core/styles';

import CircularProgress from '@material-ui/core/CircularProgress';

const useStyles = makeStyles((theme) => ({

root: {

display: 'flex',

'& > * + *': {

marginLeft: theme.spacing(2),

},

},

}));

const Spinner: FC = () => {

import { makeStyles } from '@material-ui/core/styles';

import CircularProgress from '@material-ui/core/CircularProgress';

const useStyles = makeStyles((theme) => ({

root: {

display: 'flex',

'& > * + *': {

marginLeft: theme.spacing(2),

},

},

}));

const Spinner = () => {

AFAIK we don't need to import FC

chrispinkney · 2021-01-20T03:29:25Z

src/frontend/next/src/components/SearchPage/SearchPage.tsx

@@ -0,0 +1,54 @@
+import { FC, FormEvent, useState } from 'react';


Suggested change

import { FC, FormEvent, useState } from 'react';

import { FormEvent, useState } from 'react';

chrispinkney · 2021-01-20T03:29:35Z

src/frontend/next/src/components/SearchPage/SearchPage.tsx

+  },
+}));
+
+const SearchPage: FC = () => {


Suggested change

const SearchPage: FC = () => {

const SearchPage = () => {

chrispinkney · 2021-01-20T03:43:02Z

src/frontend/next/src/components/SearchPage/SearchPage.tsx

+  // via `textParam` and `filterParam`. The <SearchBar> UI uses our internal
+  // state values, and the <SearchResults> only update on page load or when
+  // the user submits the form.
+  const [textParam = '', setTextParam] = useQueryParam('text', StringParam);


So I merged in the current master to this pr and I'm getting errors when trying to render <SearchPage />:

Error: useQueryParams must be used within a QueryParamProvider

Got any ideas?

chrispinkney · 2021-01-20T03:44:19Z

src/frontend/next/src/components/SearchPage/SearchResults.tsx

@@ -0,0 +1,85 @@
+import { FC } from 'react';


Suggested change

import { FC } from 'react';

chrispinkney · 2021-01-20T03:45:38Z

src/frontend/next/src/components/SearchPage/SearchResults.tsx

+  },
+}));
+
+const SearchResults: FC<SearchPageProps> = ({ text, filter }: SearchPageProps) => {


Suggested change

const SearchResults: FC<SearchPageProps> = ({ text, filter }: SearchPageProps) => {

const SearchResults = ({ text, filter }: SearchPageProps) => {

chrispinkney · 2021-01-20T03:46:28Z

src/frontend/next/src/components/SearchPage/SearchResults.tsx

+  if (error) {
+    return (
+      <Container className={classes.searchResults}>
+        <Box className={classes.root} boxShadow={2} marginTop={10}>


Getting Property 'root' does not exist on type 'Record<"spinner" | "searchResults", string>'. with the latest master merge, same with line 52 and 53.

But I'm getting No search results when rendering the component so I think it's fine.

chrispinkney · 2021-01-20T03:48:48Z

So that was a lot, to the point where I feel like I might be doing something wrong on my end, haha. Let me know what you think, plz n thanks!

manekenpix

LGTM 👍

chrispinkney

😍

vercel bot deployed to Preview November 26, 2020 03:59 View deployment

c3ho changed the title ~~Search page~~ Port SearchPage from Gatsby to Next.js(TS) Nov 26, 2020

manekenpix reviewed Nov 26, 2020

View reviewed changes

c3ho added Blocked Can't do this, until something else is done area: front-end labels Nov 27, 2020

c3ho self-assigned this Nov 27, 2020

rogercyyu marked this pull request as ready for review November 30, 2020 16:47

rogercyyu marked this pull request as draft November 30, 2020 16:50

vercel bot deployed to Preview December 3, 2020 18:58 View deployment

vercel bot deployed to Preview December 3, 2020 19:02 View deployment

c3ho changed the title ~~Port SearchPage from Gatsby to Next.js(TS)~~ Port SearchPage + Spinner from Gatsby to Next.js(TS) Dec 3, 2020

c3ho linked an issue Dec 3, 2020 that may be closed by this pull request

Port Spinner from Gatsby to Next.js(TS) #1479

Closed

vercel bot deployed to Preview December 3, 2020 22:07 View deployment

vercel bot deployed to Preview December 3, 2020 22:12 View deployment

c3ho marked this pull request as ready for review December 4, 2020 00:33

humphd requested review from birtony, chrispinkney and rjayroso January 19, 2021 15:45

humphd added this to the 1.5 Release milestone Jan 19, 2021

humphd added area: nextjs Nextjs related issues Priority: High labels Jan 19, 2021

chrispinkney reviewed Jan 20, 2021

View reviewed changes

c3ho added 4 commits January 20, 2021 09:53

Port SearchPage from Gatsby to Next.js(TS)

a5b57c6

updated package.json

0d7d765

changed to interface + fixes

5db2d40

missing check

e3adc78

c3ho added 5 commits January 20, 2021 09:57

Added spinner as well to PR

e458ca8

fix export

1001629

resolving swr issues

d563070

fix line 30 block

b5bb1a9

removed SearchPage for now

9858bc8

c3ho force-pushed the SearchPage branch from 49a4ec4 to 9858bc8 Compare January 20, 2021 15:02

vercel bot deployed to Preview January 20, 2021 15:02 View deployment

removed unused package

280b164

c3ho changed the title ~~Port SearchPage + Spinner from Gatsby to Next.js(TS)~~ Port Spinner from Gatsby to Next.js(TS) Jan 20, 2021

c3ho requested review from chrispinkney and manekenpix January 20, 2021 15:06

vercel bot deployed to Preview January 20, 2021 15:09 View deployment

c3ho removed the Blocked Can't do this, until something else is done label Jan 20, 2021

manekenpix approved these changes Jan 20, 2021

View reviewed changes

chrispinkney approved these changes Jan 20, 2021

View reviewed changes

chrispinkney merged commit 46f2c4c into Seneca-CDOT:master Jan 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port Spinner from Gatsby to Next.js(TS) #1462

Port Spinner from Gatsby to Next.js(TS) #1462

c3ho commented Nov 26, 2020 •

edited

Loading

manekenpix Nov 26, 2020

manekenpix Nov 26, 2020

rogercyyu commented Nov 30, 2020

humphd commented Jan 15, 2021

chrispinkney Jan 20, 2021

chrispinkney Jan 20, 2021

chrispinkney Jan 20, 2021

chrispinkney Jan 20, 2021

chrispinkney Jan 20, 2021

chrispinkney Jan 20, 2021

chrispinkney Jan 20, 2021

chrispinkney Jan 20, 2021

chrispinkney commented Jan 20, 2021

manekenpix left a comment

chrispinkney left a comment

		@@ -0,0 +1,54 @@
		import React, { FC, FormEvent, useState } from 'react';

		@@ -0,0 +1,54 @@
		import { FC, FormEvent, useState } from 'react';

	import { FC, FormEvent, useState } from 'react';
	import { FormEvent, useState } from 'react';

	const SearchResults: FC<SearchPageProps> = ({ text, filter }: SearchPageProps) => {
	const SearchResults = ({ text, filter }: SearchPageProps) => {

Port Spinner from Gatsby to Next.js(TS) #1462

Port Spinner from Gatsby to Next.js(TS) #1462

Conversation

c3ho commented Nov 26, 2020 • edited Loading

Issue This PR Addresses

Type of Change

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rogercyyu commented Nov 30, 2020

humphd commented Jan 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrispinkney commented Jan 20, 2021

manekenpix left a comment

Choose a reason for hiding this comment

chrispinkney left a comment

Choose a reason for hiding this comment

c3ho commented Nov 26, 2020 •

edited

Loading