From 6d89fb865759c401d3a076db940a5bf7bdb6423b Mon Sep 17 00:00:00 2001 From: Caleb Braaten Date: Thu, 11 Apr 2024 19:50:19 -0700 Subject: [PATCH] Completion of Form Component, Component Viewer, and Documentation --- README.md | 45 ++++++-- package-lock.json | 16 +++ package.json | 3 + solution/src/App.css | 39 +------ solution/src/App.js | 22 ++-- .../ComponentViewer/ComponentViewer.css | 5 + .../src/components/ComponentViewer/index.js | 25 +++++ solution/src/components/Form/Form.css | 18 ++++ solution/src/components/Form/index.js | 50 +++++++++ .../atoms/LocationInput/LocationInput.css | 45 ++++++++ .../components/atoms/LocationInput/index.js | 51 +++++++++ .../components/atoms/NameInput/NameInput.css | 89 +++++++++++++++ .../src/components/atoms/NameInput/index.js | 101 ++++++++++++++++++ .../atoms/RenderTable/RenderTable.css | 35 ++++++ .../src/components/atoms/RenderTable/index.js | 33 ++++++ 15 files changed, 517 insertions(+), 60 deletions(-) create mode 100644 solution/src/components/ComponentViewer/ComponentViewer.css create mode 100644 solution/src/components/ComponentViewer/index.js create mode 100644 solution/src/components/Form/Form.css create mode 100644 solution/src/components/Form/index.js create mode 100644 solution/src/components/atoms/LocationInput/LocationInput.css create mode 100644 solution/src/components/atoms/LocationInput/index.js create mode 100644 solution/src/components/atoms/NameInput/NameInput.css create mode 100644 solution/src/components/atoms/NameInput/index.js create mode 100644 solution/src/components/atoms/RenderTable/RenderTable.css create mode 100644 solution/src/components/atoms/RenderTable/index.js diff --git a/README.md b/README.md index 6844294..c9e60ae 100644 --- a/README.md +++ b/README.md @@ -1,17 +1,46 @@ # react-interview-q1 -## Instructions +## Summary + +This was a coding challenge. To see the original prompt, scroll down to the "Original Prompt" heading. Nothing is edited below that. Above that, under the "My Implementation" heading, I will give some explanations as to why I made the choices I did. This was a really fun challenge because I actually used it as an opportunity to work outside of React's data model and leverage RxJS. This came with a couple fun edge cases to solve for and I am happy to nerd out about the solutions I came up with - Feel free to reach out! + +## My Implementation + +### Styling Notes + +1. CSS follows in the footsteps of create-react-app and are external files pulled in via imports. There are no frameworks or libraries used for styling. Modern features like Flexbox and container queries are used to make the form responsive. Components are styled with component name prefixes to reduce the chance of class name collisions. +2. I wanted to easily adjust the form components size when styling, so I created the `ComponentViewer` component to wrap the form. This provides a slider to adjust the width of the nested component (in this case `Form`) dynamically to see how it looks at different sizes without opening dev tools. + +### Component Composition Notes + +1. The job description this coding challenge was for listed RxJS as a requirement. I hadn't used RxJS before, so I used this as an opportunity to learn it. Because of this, I broke up the form into smaller components and chose not to pass data through props. Instead, I used RxJS `Subjects` to push data from the child components to anyone who would listen. In this case, the parent component, `Form`, listens to the `NameInput` and `LocationInput` components found in `src/components/atoms`. +2. The `RenderTable` component is kept pretty simple and more closely follows idiomatic React. It receives data from the `Form` component and iterates through to render the table rows. There is also a minimum of 5 rows that will always be rendered. In order to achieve this though, key's are explicitly not provided so that React doesn't accidentally persist data when clearing. This is a trade off, but I think the user experience of seeing the table is more important than the minuscule performance loss. If this were populated with a large amount of data, I would reconsider this decision. While there would be a technical solution to achieve both I didn't want to sacrifice code readability for a small performance gain. + +### Just Cool Stuff + +1. `LocationInput` is more like a Solid component in that once React renders it, there is no reason it needs to be rendered agin (YAY Performance!) and it relies on 'signals'. This is because the data is fetched from the API on mount and populated initially. Alongside this action, a listener is mounted so that whenever there is a change on the select element, it is dispatched straight to the `Subject` which makes the selection available to any other component outside of Reacts rendering cycle. +2. `NameInput` is a little more complex. Similar to `LocationInput`, it pushes its state changes through a `Subject` making it available anywhere. Before we can do this though, we need to validate the input. Name validation is an async operation though and if we just validate every keystroke, we won't know if the response we get back is for the most recent keystroke. To solve this, I used an RxJS Operator to ignore all validation responses except the most recent. This eliminated the risk of the user seeing a false positive or negative caused by an older request coming back after a newer one. + +This still has a massive problem though because the response to the most recent request could come back after the user has changed the input again. This is because the user could have typed in a new character and we are still waiting for that response to be received. There is no way to verify that the response is for the current string in the input box. The API is not available for us to make changes to as well so I solved this by wrapping every request in a new promise that includes the name we asked to validate. This way, we can save the name in a ref that is kept in sync with the input box and compare that with the validation response. If they are the same, we know the user has stopped typing and we can dispatch the valid name to the subject. + +To make this a little better of a user experience, and because all the building blocks were there, I added additional states for validation. Every time a new key is pressed a `pending` is dispatched to the subject and a message is shown to the user. If the response comes back and the name is the same as the current input, the message is updated to reflect the validity of the name and an additional message is dispatched to the `subject` to inform other components that the name is valid or not. + +This is actually consumed by the `Form` component to enable or disable the submit button only when the name is guaranteed to be valid. (using the Mock API of course, no way to solve the two generals problem here) + +## Original Prompt + +### Instructions Fork this repo first into your own github account. Make sure to thoroughly read the instructions and implement the react component to meet the provided requirements. Send back a link to your cloned repo. You are expected to make implementation choices around customer experience and efficiency. Please make sure to explain your choices in comments. -## Requirements +### Requirements Please build the following form component ![form component mock](./mock.png) -* Name input should be validated using the provided mock API to check whether the chosen name is taken or not. -* Name input should be validated as the user is typing. -* Location dropdown options should be fetched using the provided mock API. -* Component should have a responsive layout -* Component should be appropriately styled -* Unit tests are not required \ No newline at end of file +- Name input should be validated using the provided mock API to check whether the chosen name is taken or not. +- Name input should be validated as the user is typing. +- Location dropdown options should be fetched using the provided mock API. +- Component should have a responsive layout +- Component should be appropriately styled +- Unit tests are not required diff --git a/package-lock.json b/package-lock.json index e84b969..5c30187 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,6 +8,9 @@ "name": "react-interview-q1", "version": "1.0.0", "license": "MIT", + "dependencies": { + "rxjs": "^7.8.1" + }, "devDependencies": { "create-react-app": "^5.0.1" } @@ -453,6 +456,14 @@ "rimraf": "bin.js" } }, + "node_modules/rxjs": { + "version": "7.8.1", + "resolved": "https://registry.npmjs.org/rxjs/-/rxjs-7.8.1.tgz", + "integrity": "sha512-AA3TVj+0A2iuIoQkWEK/tqFjBq2j+6PO6Y0zJcvzLAFhEFIO3HL0vls9hWLncZbAAbK0mar7oZ4V079I/qPMxg==", + "dependencies": { + "tslib": "^2.1.0" + } + }, "node_modules/safe-buffer": { "version": "5.1.2", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz", @@ -626,6 +637,11 @@ "url": "https://github.com/sponsors/isaacs" } }, + "node_modules/tslib": { + "version": "2.6.2", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.6.2.tgz", + "integrity": "sha512-AEYxH93jGFPn/a2iVAwW87VuUIkR1FVUKB77NwMF7nBTDkDrrT/Hpt/IrCJ0QXhW27jTBDcf5ZY7w6RiqTMw2Q==" + }, "node_modules/uid-number": { "version": "0.0.6", "resolved": "https://registry.npmjs.org/uid-number/-/uid-number-0.0.6.tgz", diff --git a/package.json b/package.json index e643538..09f3f81 100644 --- a/package.json +++ b/package.json @@ -17,5 +17,8 @@ "homepage": "https://github.com/MasterRyd3l/react-interview-q1#readme", "devDependencies": { "create-react-app": "^5.0.1" + }, + "dependencies": { + "rxjs": "^7.8.1" } } diff --git a/solution/src/App.css b/solution/src/App.css index 74b5e05..3c7171b 100644 --- a/solution/src/App.css +++ b/solution/src/App.css @@ -1,38 +1,5 @@ .App { - text-align: center; -} - -.App-logo { - height: 40vmin; - pointer-events: none; -} - -@media (prefers-reduced-motion: no-preference) { - .App-logo { - animation: App-logo-spin infinite 20s linear; - } -} - -.App-header { - background-color: #282c34; - min-height: 100vh; - display: flex; - flex-direction: column; - align-items: center; - justify-content: center; - font-size: calc(10px + 2vmin); - color: white; -} - -.App-link { - color: #61dafb; -} - -@keyframes App-logo-spin { - from { - transform: rotate(0deg); - } - to { - transform: rotate(360deg); - } + display: flex; + flex-direction: column; + align-items: center; } diff --git a/solution/src/App.js b/solution/src/App.js index 3784575..4b77bf0 100644 --- a/solution/src/App.js +++ b/solution/src/App.js @@ -1,23 +1,13 @@ -import logo from './logo.svg'; -import './App.css'; +import ComponentViewer from "./components/ComponentViewer"; +import Form from "./components/Form"; +import "./App.css"; function App() { return (
-
- logo -

- Edit src/App.js and save to reload. -

- - Learn React - -
+ +
+
); } diff --git a/solution/src/components/ComponentViewer/ComponentViewer.css b/solution/src/components/ComponentViewer/ComponentViewer.css new file mode 100644 index 0000000..55b51f6 --- /dev/null +++ b/solution/src/components/ComponentViewer/ComponentViewer.css @@ -0,0 +1,5 @@ +/* Style Injection to size component +to that of what the slider specifies*/ +.ComponentViewer div { + box-sizing: border-box; +} diff --git a/solution/src/components/ComponentViewer/index.js b/solution/src/components/ComponentViewer/index.js new file mode 100644 index 0000000..44b23dd --- /dev/null +++ b/solution/src/components/ComponentViewer/index.js @@ -0,0 +1,25 @@ +import React, { useState } from "react"; + +export default function ComponentViewer({ name, children }) { + const [width, setWidth] = useState(450); + + return ( + <> +

{name}

+ + setWidth(e.target.value)} + > +
+ + {/* Set the container width to the slider definition */} +
{children}
+ + ); +} diff --git a/solution/src/components/Form/Form.css b/solution/src/components/Form/Form.css new file mode 100644 index 0000000..aa7ac21 --- /dev/null +++ b/solution/src/components/Form/Form.css @@ -0,0 +1,18 @@ +.Form-container { + border: solid black; + width: 100%; + container-type: inline-size; + box-sizing: border-box; +} + +.Form-actions { + display: flex; + justify-content: flex-end; + gap: 20px; + margin: 10px; +} + +.Form-container > form { + max-width: 515px; + margin: 1.5rem auto; +} diff --git a/solution/src/components/Form/index.js b/solution/src/components/Form/index.js new file mode 100644 index 0000000..10c8de6 --- /dev/null +++ b/solution/src/components/Form/index.js @@ -0,0 +1,50 @@ +import NameInput, { selectedName } from "../atoms/NameInput"; +import LocationInput, { selectedLocation } from "../atoms/LocationInput"; +import RenderTable from "../atoms/RenderTable"; +import "./Form.css"; +import { useEffect, useRef, useState } from "react"; + +export default function Form() { + const [table, setTable] = useState([]); + const [isNameValid, setIsNameValid] = useState(false); + const locationRef = useRef(); + const nameRef = useRef(); + + useEffect(() => { + selectedLocation.subscribe((value) => { + locationRef.current = value; + }); + + selectedName.subscribe((value) => { + nameRef.current = value; + value.isValid === true ? setIsNameValid(true) : setIsNameValid(false); + }); + }, []); + + function addToTable() { + setTable([ + { name: nameRef.current.name, location: locationRef.current }, + ...table, + ]); + } + + function clearTable() { + setTable([]); + } + + return ( +
+ e.preventDefault()}> + + +
+ + +
+ + +
+ ); +} diff --git a/solution/src/components/atoms/LocationInput/LocationInput.css b/solution/src/components/atoms/LocationInput/LocationInput.css new file mode 100644 index 0000000..5dd71d5 --- /dev/null +++ b/solution/src/components/atoms/LocationInput/LocationInput.css @@ -0,0 +1,45 @@ +@container (width > 0px) { + .LocationInput { + container: inline-size; + max-width: 515px; + margin: 5px 5px 25px 5px; + gap: 5px; + } + + .LocationInput label { + display: block; + } + + .LocationInput > select { + box-sizing: border-box; + width: 100%; + } +} + +@container (width > 300px) { + .LocationInput { + display: flex; + } + + .LocationInput label { + flex: 0 80px; + display: flex; + justify-content: flex-end; + padding-top: 5px; + padding-bottom: 5px; + margin-right: 15px; + } + + .LocationInput > select { + flex: 1; + justify-content: flex-start; + font-size: 1rem; + padding: 0px; + } +} + +@container (width > 430px) { + .LocationInput label { + font-size: 1.2rem; + } +} diff --git a/solution/src/components/atoms/LocationInput/index.js b/solution/src/components/atoms/LocationInput/index.js new file mode 100644 index 0000000..27c7709 --- /dev/null +++ b/solution/src/components/atoms/LocationInput/index.js @@ -0,0 +1,51 @@ +import { useState, useEffect } from "react"; +import { getLocations } from "../../../mock-api/apis.js"; +import "./LocationInput.css"; +import { Subject } from "rxjs"; + +// Fetches location options on mount +// Shares currently selected location with any listeners (anywhere!) +// Finite number of renders -> no pass by props re-rendering +export default function LocationInput() { + const [options, setOptions] = useState([]); + + // on component mount (Only runs once!) + useEffect(() => { + // Fetch selectable locations + getLocations().then((locations) => { + // push the first location to the selectedLocation subject + // This is how we share the selected location with listeners + selectedLocation.next(locations[0]); + + // Render the location options and save to state for re-render + // Options are static -> no need to re-render after this. + setOptions( + locations.map((option) => { + return ( + + ); + }), + ); + }); + }, []); + + return ( +
+ + +
+ ); +} + +export const selectedLocation = new Subject(); diff --git a/solution/src/components/atoms/NameInput/NameInput.css b/solution/src/components/atoms/NameInput/NameInput.css new file mode 100644 index 0000000..8a4c417 --- /dev/null +++ b/solution/src/components/atoms/NameInput/NameInput.css @@ -0,0 +1,89 @@ +@container (width > 0px) { + .NameInput { + container: inline-size; + max-width: 515px; + margin: 5px 5px 25px 5px; + gap: 5px; + } + + .NameInput label { + display: block; + } + + .NameInput > span > input { + box-sizing: border-box; + width: 100%; + } + + .NameInput > span:nth-child(2) > span { + height: 16px; + } + + #Form-error { + color: red; + } + + #Form-pending { + color: gold; + } + + #Form-valid { + color: green; + } +} + +@container (width > 300px) { + .NameInput { + display: flex; + justify-content: space-between; + flex-direction: column; + } + + /* .NameInput label { + padding-top: 5px; + padding-bottom: 5px; + margin-left: 5px; + width: 80px; + text-align: right; + display: inline-block; + } */ + + .NameInput span { + display: flex; + } + + .NameInput > span > label, + .NameInput > span > span { + flex: 0 80px; + display: flex; + justify-content: flex-end; + padding-top: 5px; + padding-bottom: 5px; + margin-right: 15px; + } + + .NameInput > span > input, + .NameInput > span > span:nth-child(2) { + flex: 1; + justify-content: flex-start; + margin-left: 5px; + font-size: 1rem; + padding: 0px; + } + + .NameInput > span:nth-child(2) > span { + font-size: 1rem; + height: 1rem; + } +} + +@container (width > 430px) { + .NameInput label, + .NameInput > span > input { + font-size: 1.2rem; + } + + .NameInput > span > span:nth-child(2) { + font-size: 1.5rem; + } +} diff --git a/solution/src/components/atoms/NameInput/index.js b/solution/src/components/atoms/NameInput/index.js new file mode 100644 index 0000000..ca1c198 --- /dev/null +++ b/solution/src/components/atoms/NameInput/index.js @@ -0,0 +1,101 @@ +import "./NameInput.css"; +import { isNameValid as getNameValidity } from "../../../mock-api/apis.js"; +import { useState, useEffect, useRef } from "react"; +import { Subject, from, switchAll } from "rxjs"; + +// The native api does not include the name that was checked +// This can cause an incorrect determination of the validity +// of the name if the name is changed before the api call is +// completed. This function wraps the api call to include the +// name that was checked to compare with the current name. +function isNameValid(name) { + return new Promise((resolve) => { + getNameValidity(name).then((value) => { + resolve({ name: name, isValid: value }); + }); + }); +} + +// Checks if the name is valid +// Shares the name with any listeners (anywhere!) +// Finite number of renders -> no pass by props re-rendering +// Guarantees the following +// A subscriber will always get the latest name +// A subscriber will always know if the 'settled' name is valid +// A subscriber can read if a name has not 'settled' and is still pending validation +export default function NameInput() { + const [name, setName] = useState(""); + const [isError, setIsError] = useState(false); + const [isPending, setIsPending] = useState(false); + const [isValid, setIsValid] = useState(false); + const nameRef = useRef(name); + + // Anytime name is updated + useEffect(() => { + if (name.length > 0) { + // Send the name to the api to check if it is valid (async) + let observable = from(isNameValid(name)); + latestObservable.next(observable); + + // Update the nameRef to validate that the name has not changed + // after the above api call was made + nameRef.current = name; + + // Broadcast the latest name and it's pending status + selectedName.next({ name: name, isValid: "pending" }); + setIsPending(true); + setIsError(false); + setIsValid(false); + } else { + selectedName.next({ name: "", isValid: "false" }); + setIsPending(false); + setIsError(false); + setIsValid(false); + } + }, [name]); + + // Mount the observable listener on component mount + useEffect(() => { + // Only read the most recent API response and ignore the rest + latestObservable.pipe(switchAll()).subscribe((nameObject) => { + // Check that the name has not changed since the api call was made + if (nameObject.name === nameRef.current) { + setIsPending(false); + // Show error if name is invalid + nameObject.isValid ? setIsError(false) : setIsError(true); + nameObject.isValid ? setIsValid(true) : setIsValid(false); + + // Broadcast the latest name and it's validity + selectedName.next(nameObject); + } + }); + }, []); + + return ( +
+ + + { + setName(e.target.value); + }} + /> + + + {/* Spacer */} + {isError ? ( + This name has already been taken + ) : null} + {isPending ? Checking... : null} + {isValid ? Available! : null} + +
+ ); +} + +const latestObservable = new Subject(); +export const selectedName = new Subject(); diff --git a/solution/src/components/atoms/RenderTable/RenderTable.css b/solution/src/components/atoms/RenderTable/RenderTable.css new file mode 100644 index 0000000..d257dda --- /dev/null +++ b/solution/src/components/atoms/RenderTable/RenderTable.css @@ -0,0 +1,35 @@ +.Form-table { + border-top: solid black 2px; + border-spacing: 0; + empty-cells: show; + width: 100%; + max-width: 515px; +} + +.Form-table th:nth-child(1), +.Form-table td:nth-child(1) { + border-right: solid black 2px; +} + +.Form-table thead tr { + background-color: lightgray; + text-align: left; + border: solid lightgray 2px; +} + +.Form-table tbody tr:nth-child(even) { + background-color: #ffffff; + height: 22px; +} + +.Form-table tbody tr:nth-child(odd) { + background-color: #f2f2f2; + height: 22px; +} + +@container (width > 515px) { + .Form-table { + border: solid black 2px; + margin: 25px auto; + } +} diff --git a/solution/src/components/atoms/RenderTable/index.js b/solution/src/components/atoms/RenderTable/index.js new file mode 100644 index 0000000..9fc5213 --- /dev/null +++ b/solution/src/components/atoms/RenderTable/index.js @@ -0,0 +1,33 @@ +import "./RenderTable.css"; + +export default function RenderTable({ records }) { + const tableRows = records.map((record) => { + return ( + + {record.name} + {record.location} + + ); + }); + + while (tableRows.length < 5) { + tableRows.push( + + + + , + ); + } + + return ( + + + + + + + + {tableRows} +
NameLocation
+ ); +}