[next] Issue 1470 - Port SearchBar to NEXT.js #1581

PedroFonsecaDEV · 2021-01-21T10:34:55Z

Issue This PR Addresses

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

I don't know if we need to merge something else related to our styles and theme or if I need to work on this particular style, but this is the result that I'm getting followed by the messages on the console.

Console:

Warning: Prop `className` did not match. Server: "MuiSvgIcon-root makeStyles-infoButton-11" Client: "MuiSvgIcon-root makeStyles-infoButton-12"
    in svg (created by ForwardRef(SvgIcon))
    in ForwardRef(SvgIcon) (created by WithStyles(ForwardRef(SvgIcon)))
    in WithStyles(ForwardRef(SvgIcon)) (created by ForwardRef(InfoOutlinedIcon))
    in ForwardRef(InfoOutlinedIcon)
    in ForwardRef(InfoOutlinedIcon) (at SearchHelp.tsx:68)
    in ForwardRef(Tooltip) (created by WithStyles(ForwardRef(Tooltip)))
    in WithStyles(ForwardRef(Tooltip)) (created by WithStyles(WithStyles(ForwardRef(Tooltip))))
    in WithStyles(WithStyles(ForwardRef(Tooltip))) (at SearchHelp.tsx:27)
    in SearchHelp (at SearchBar.tsx:111)
    in div (created by ForwardRef(Grid))
    in ForwardRef(Grid) (created by WithStyles(ForwardRef(Grid)))
    in WithStyles(ForwardRef(Grid)) (at SearchBar.tsx:98)
    in form (created by ForwardRef(Paper))
    in ForwardRef(Paper) (created by WithStyles(ForwardRef(Paper)))
    in WithStyles(ForwardRef(Paper)) (at SearchBar.tsx:97)
    in div (created by Styled(MuiBox))
    in Styled(MuiBox) (at SearchBar.tsx:96)
    in SearchBar (at pages/index.tsx:20)
    in Home (at _app.tsx:21)
    in UserProvider (at _app.tsx:20)
    in App
    in ErrorBoundary (created by ReactDevOverlay)
    in ReactDevOverlay (created by Container)
    in Container (created by AppContainer)
    in AppContainer
    in Root
    in StrictMode
printWarning @ react-dom.development.js?61bb:88
error @ react-dom.development.js?61bb:60
warnForPropDifference @ react-dom.development.js?61bb:5720
diffHydratedProperties @ react-dom.development.js?61bb:6515
hydrateInstance @ react-dom.development.js?61bb:7691
prepareToHydrateHostInstance @ react-dom.development.js?61bb:16611
completeWork @ react-dom.development.js?61bb:18964
completeUnitOfWork @ react-dom.development.js?61bb:22192
performUnitOfWork @ react-dom.development.js?61bb:22165
workLoopSync @ react-dom.development.js?61bb:22130
performSyncWorkOnRoot @ react-dom.development.js?61bb:21756
scheduleUpdateOnFiber @ react-dom.development.js?61bb:21188
updateContainer @ react-dom.development.js?61bb:24373
eval @ react-dom.development.js?61bb:24758
unbatchedUpdates @ react-dom.development.js?61bb:21903
legacyRenderSubtreeIntoContainer @ react-dom.development.js?61bb:24757
hydrate @ react-dom.development.js?61bb:24823
renderReactElement @ index.tsx?8abf:518
doRender @ index.tsx?8abf:776
_callee2$ @ index.tsx?8abf:413
tryCatch @ runtime.js?96cf:63
invoke @ runtime.js?96cf:293
eval @ runtime.js?96cf:118
asyncGeneratorStep @ asyncToGenerator.js?c973:3
_next @ asyncToGenerator.js?c973:25
eval @ asyncToGenerator.js?c973:32
eval @ asyncToGenerator.js?c973:21
_render @ index.js:515
render @ index.js:452
eval @ next-dev.js?53bc:85
eval @ fouc.js?937a:14
requestAnimationFrame (async)
displayContent @ fouc.js?937a:5
eval @ next-dev.js?53bc:84
Promise.then (async)
eval @ next-dev.js?53bc:31
eval @ next-dev.js?53bc:31
./node_modules/next/dist/client/next-dev.js @ main.js?ts=1611224705998:956
__webpack_require__ @ webpack.js?ts=1611224705998:873
checkDeferredModules @ webpack.js?ts=1611224705998:46
webpackJsonpCallback @ webpack.js?ts=1611224705998:33
(anonymous) @ webpack.js?ts=1611224705998:1015
(anonymous) @ webpack.js?ts=1611224705998:1023

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)

tonyvugithub · 2021-01-21T13:36:53Z

src/frontend/next/src/components/SearchBar.tsx

+
+type searchBarProps = {
+  text: string;
+  onTextChange: Function;


for onTextChange should we be clearer than just Function. It is an onChange event handler and I believe in TS you have ChangeEventHandler/EventHandler type too. Whats your thought?

It should be changed to a ChangeEvent function that returns void
Since it's for an input element, we can type it to HTMLInputElement

Let's do it in a follow-up PR, because we are drilling a lot of stuff here. After getting everything working we will need to create an interface to SearchPage's components and a context. These functions are connected to how we build the URL query of a search. So I believe that we need to be care full with that. It's working now. Let's keep this for now.

I'm opening an issue for the next step (interface and context) now.

tonyvugithub · 2021-01-21T13:52:24Z

@PedroFonsecaDEV : Your error is surprising as I got it deal with in this PR #1550

HyperTHD · 2021-01-21T14:00:28Z

src/frontend/next/src/components/SearchBar.tsx

+  text: string;
+  onTextChange: Function;
+  filter: string;
+  onFilterChange: Function;


similar thing here, we should probably be more specific by specifying any parameters it may take or "()" for nothing, as well as the return type for this function

Also, this is for the FilterChange and TextChange functions, text and filter are fine

HyperTHD

Once SearchInput gets merged, you should probably rebase this so you can pick it up, would prefer to avoid duplicate components in 2 PRs

HyperTHD

@c3ho is gonna rebase this PR and as per your comment here, we'll see what we can do to fix this

PedroFonsecaDEV added 5 commits January 21, 2021 03:46

Create SearchBar.tsx

a0b75f9

imports + searchinput

dfb43be

createStyles

7f57108

import react changeEvent

bb0ab58

fix event type

d89015b

vercel bot deployed to Preview January 21, 2021 10:35 View deployment

PedroFonsecaDEV requested review from birtony, humphd and tonyvugithub January 21, 2021 10:35

PedroFonsecaDEV self-assigned this Jan 21, 2021

PedroFonsecaDEV added the area: nextjs Nextjs related issues label Jan 21, 2021

PedroFonsecaDEV added this to the 1.5 Release milestone Jan 21, 2021

This was referenced Jan 21, 2021

Fix #1470: Port SearchBar from Gatsby -> NextJS #1503

Closed

[next] Issue 1500 - Port SearchInput to NEXT.js #1580

Merged

tonyvugithub suggested changes Jan 21, 2021

View reviewed changes

HyperTHD reviewed Jan 21, 2021

View reviewed changes

tonyvugithub approved these changes Jan 22, 2021

View reviewed changes

HyperTHD requested changes Jan 22, 2021

View reviewed changes

HyperTHD self-requested a review January 22, 2021 17:47

HyperTHD approved these changes Jan 22, 2021

View reviewed changes

HyperTHD merged commit 0c9c86e into Seneca-CDOT:master Jan 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[next] Issue 1470 - Port SearchBar to NEXT.js #1581

[next] Issue 1470 - Port SearchBar to NEXT.js #1581

PedroFonsecaDEV commented Jan 21, 2021 •

edited

Loading

tonyvugithub Jan 21, 2021

HyperTHD Jan 21, 2021

PedroFonsecaDEV Jan 22, 2021

PedroFonsecaDEV Jan 22, 2021

tonyvugithub commented Jan 21, 2021

HyperTHD Jan 21, 2021

HyperTHD Jan 21, 2021

HyperTHD left a comment

HyperTHD left a comment

[next] Issue 1470 - Port SearchBar to NEXT.js #1581

[next] Issue 1470 - Port SearchBar to NEXT.js #1581

Conversation

PedroFonsecaDEV commented Jan 21, 2021 • edited Loading

Issue This PR Addresses

Type of Change

Description

Console:

Checklist

tonyvugithub Jan 21, 2021

Choose a reason for hiding this comment

HyperTHD Jan 21, 2021

Choose a reason for hiding this comment

PedroFonsecaDEV Jan 22, 2021

Choose a reason for hiding this comment

PedroFonsecaDEV Jan 22, 2021

Choose a reason for hiding this comment

tonyvugithub commented Jan 21, 2021

HyperTHD Jan 21, 2021

Choose a reason for hiding this comment

HyperTHD Jan 21, 2021

Choose a reason for hiding this comment

HyperTHD left a comment

Choose a reason for hiding this comment

HyperTHD left a comment

Choose a reason for hiding this comment

PedroFonsecaDEV commented Jan 21, 2021 •

edited

Loading