Closed rzgry closed 4 years ago
[x] The index.js file is the main entry point of the application as it coincides with the index.html - Not exactly accurate, Create React App is configured to use index.js as the entry point of your application not bc it shares the same name with index.html.
[x] To simplify the guide it might be better to just remove the serviceWorker as it seems out of scope of "consuming rest data w/ react"
[x] Instead of using a React.Fragment might be simpler just to use a div to avoid having to explain what a fragment is.
[x] if you are going to use object spread syntax may want to explain what it is doing or link to the MDN page as it is relatively new (ES2018) (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals) Ie. posts.push({ ...rest, ...album });
[x] Seems like none of the styles in App.css are actually used anywhere, can prob just delete or change to add some stylings that are used.
[x] The test you provide in the guide will fail. The app does not render a learn react
link anywhere. Could change to be /Artist Web Service/
test('renders learn react link', () => {
const { getByText } = render(<App />);
const linkElement = getByText(/learn react/i);
expect(linkElement).toBeInTheDocument();
});
Addressed the feedback @rzgry, thanks. The draft site is updating.
Testing the React client
not mentioning how to run tests, since we are not concerned about testing the application in the guide, this section serves more as information rather than steps on how to run test, which is also why it just references the App.test.js file as it is the default from the configuration.
[x] Add .DS_store to gitignore and remove from repo.
[x] "The application uses the Create React App prebuilt configuration to design the modern single-page React application." I would probably change "design" to "set up" or something similar.
[x] Not a big fan of this paragraph. Should prob mention that these are frontend dependencies and not for liberty.
Note: all the other dependencies that are installed in the project will show up in src/main/frontend/src/package.json that have already been generated for you through the create-react-app configuration. There are also pre-coded .css files that can be found in src/main/frontend/src/Styles.
Something like:
Note: the dependencies for the React frontend can be found in src/main/frontend/src/package.json
and are installed before building the frontend by the frontend-maven-plugin
configured in the pom.xml. Additionally there are also some provided css stylesheets that can be found in src/main/frontend/src/Styles.
.
The front end of your application uses Node.js to build your React code. The Maven project is configured for you to install Node.js, and populate the production files to the web content of your application.
[x] maybe should mention it installs a local copy of node so it will not effect any existing installs. - could maybe link to maven frontend plugin docs.
[x] Node.js is a JavaScript runtime that is used for developing networking applications -> Node.JS is a server side JavaScript runtime.
[x] Its convenient package manager, npm, is used to execute the React scripts found in the package.json -> the React build scripts.
[x] This is where your code is always going to be run. -> Seems weird, Could be something like "This is the entry point of where your code is run in the browser".
[ ] The Virtual DOM improves the performance of your web application as it plays a crucial role in the rendering process. -> I would just take this line out. Doesn't really say what the vdom does and seems out of scope for this guide.
[x] The purpose in using the render method is to display the specified HTML code inside the root element by rendering the App element on a DOM node. -> The render method takes an html DOM element and tells ReactDOM to render your React application inside of this DOM element.
[x] Next, you’ll create the ArtistTable component that will define the states and properties of your React webpage. -> ...create the ArtistTable component that will fetch data from your backend and render it in a table.
[x] The getArtistInfo() function will use the Axios API. You also need to verify that the component was mounted properly. This is done through the componentDidMount() lifecycle method as it runs when a component is rendered. -> The getArtistInfo() function will use the Axios API to fetch data from your backend. It will be called when the ArtistTable is rendered to the page using the componentDidMount React lifecycle method.
[ ] You don't explain how this.setState works or how state in react works. Might not be needed to explain it in detail in the guide but maybe link to the react docs about it.
[x] Once the state is updated, your component will mount all the children elements and component instances to the native UI. You will now have access to the native UI, and will be able to access to your children references, with the ability to trigger a new render pass. -> This paragraph is weird. Are you trying to say after the state has been updated. The component will re-render and display the artist data in the table.
import React from 'react';
import { render } from '@testing-library/react';
import App from './Components/App';
test('renders learn react link', () => {
const { getByText } = render(<App />);
const linkElement = getByText(/Artist Web Service/i);
expect(linkElement).toBeInTheDocument();
});
``` -> still has reference to learn react link that doesnt exist in the test description. Change linkElement to titleElement or something similar.
At a high-level in this section it does explain the idea of the VDOM and the section ends off with a link to the documentation for more information. It helps to keep it as there is content on the rendering process.
The Virtual DOM improves the performance of your web application as it plays a crucial role in the rendering process. -> I would just take this line out. Doesn't really say what the vdom does and seems out of scope for this guide.
I believe we did not go into too much detail about this.setState or how states work as it is just referencing key JavaScript syntax in the file.
You don't explain how this.setState works or how state in react works. Might not be needed to explain it in detail in the guide but maybe link to the react docs about it.
Yes essentially, I updated the paragraph.
Once the state is updated, your component will mount all the children elements and component instances to the native UI. You will now have access to the native UI, and will be able to access to your children references, with the ability to trigger a new render pass. -> This paragraph is weird. Are you trying to say after the state has been updated. The component will re-render and display the artist data in the table.
Further feedback discussed and addressed.
Some quick thoughts. Will look at it in more detail later.
[x]
contains a list of artists, their albums and other ambiguous artist features by using an
- i dont think ambiguous is the right word here.[x]
you will then present this data using a ReactJS element in the form of a paginated table.
- present this data using a ReactJS paginated table component.[x] name and artist dont match for this entry in artist.json but match for all the others.
[x] * The front end of your application uses Node.js to execute your React code. - to build your react code
[x] running mvn liberty dev in the start directory created 4K+ untracked files in
src/main/frontend/node/
. Prob want to ignore these or configure it to install elsewhere? the frontend-plugin-docs seem to show them usingtarget
https://github.com/eirslett/frontend-maven-plugin#installation-directoryBefore starting to fetch data and build the artists table. I think it would be a good idea to build a simple hello world component in App.js that just displays "Hello world" and get users to run mvn process-resources and open browser to 9080 so they can see their page is working.
Testing the React client - Does not say how to run the tests?