-
Notifications
You must be signed in to change notification settings - Fork 1
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
React server #27
React server #27
Conversation
… the app file to allow for commit #26
@FarahZaqout
you can read his comment is these issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some comments
I'm not sure if it's essential or not
client/public/index.html
Outdated
work correctly both with client-side routing and a non-root public URL. | ||
Learn how to configure a non-root public URL by running `npm run build`. | ||
--> | ||
<title>React App</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I Think we should change this title
into a real one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah.
client/public/index.html
Outdated
<link rel="shortcut icon" href="%PUBLIC_URL%/favicon.ico"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no"> | ||
<meta name="theme-color" content="#000000"> | ||
<!-- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No needs for these commented lines.
what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
import React from 'react'; | ||
import ReactDOM from 'react-dom'; | ||
import App from './App'; | ||
import * as serviceWorker from './serviceWorker'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to make it off-line app?
if not so I think no needs for serviceWorker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good to try and make it a progressive app.
Yeah, a lot of people are pushing towards this. However, for some reason airbnb refuses to buckle. |
Maybe I should just disable the rule in eslint? |
c3866be
create react server with create-react-app
#26