Update antd and styled components #18
Conversation
…-scaffold into update_antd_and_styled_components
jackblanc
left a comment
jackblanc
left a comment
There was a problem hiding this comment.
Looks great! Left a few suggestions
antd.customize.less
Outdated
| @@ -0,0 +1,3 @@ | |||
| @link-color: #1890ff; | |||
| @primary-color: #1da57a; | |||
| @font-family: 'IBM Plex Sans', sans-serif; No newline at end of file | |||
antd.customize.less
Outdated
| @@ -0,0 +1,3 @@ | |||
| @link-color: #1890ff; | |||
There was a problem hiding this comment.
We should define the colors with descriptive names as their own variables, and set @link-color: @our-variable-color
| const CracoLessPlugin = require('craco-less'); | ||
|
|
||
| const CracoAntDesignPlugin = require('craco-antd'); | ||
| process.env.BROWSER = "none"; |
There was a problem hiding this comment.
This is required so nodemon doesnt constantly open a new tab in the browser every time it detects a change
src/App.tsx
Outdated
| import { BrowserRouter as Router, Route, Switch } from 'react-router-dom'; | ||
| import { Helmet } from 'react-helmet'; | ||
|
|
||
| // Import antd stylesheets |
| @@ -0,0 +1,2 @@ | |||
| export const PRIMARY = '#1da57a'; | |||
There was a problem hiding this comment.
Same comment as less file, lets define the colors separately and assign values here to prevent confusion
There was a problem hiding this comment.
Im a bit confused by what you mean by this? Can you elaborate and suggest what I should change? I dont think its possible to share one common file that will sync across both the antd less styles and colors as JS values.
src/components/index.tsx
Outdated
| @@ -0,0 +1,32 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
What's this file for? Common utility components?
There was a problem hiding this comment.
Yea, I'd probably just make a LinkButton component and then have a styles folder serperate from components for styles like ContentContainer.
There was a problem hiding this comment.
I can move LinkButton out into its own component folder
src/components/navbar/NavBar.tsx
Outdated
| <NavRow align="middle"> | ||
| <Col flex={1}> | ||
| <Link to="/"> | ||
| <Logo |
There was a problem hiding this comment.
Let's remove the logo from the frontend-scaffold
There was a problem hiding this comment.
Ok, this will break a bunch of CSS though
| `; | ||
|
|
||
| const ExampleCol = styled(Col)` | ||
| border: 1px #1890ff solid; |
There was a problem hiding this comment.
Can we access our variables here?
There was a problem hiding this comment.
You'd have to pull them into a ts file, not a less file.
There was a problem hiding this comment.
But, you should be using constants.
src/components/navbar/NavBar.tsx
Outdated
| const Logo = styled.img` | ||
| float: right; | ||
| `; | ||
| const LogoText = styled(Text)` |
There was a problem hiding this comment.
Probably isn't necessary for the scaffold, but I would move all typography to that styles folder and always use your typography styled components for any text. Keeps the site more consistent.
There was a problem hiding this comment.
I might just remove most of the styling @anirudh723 did for the navbar for the frontend scaffold then
| `; | ||
| const UserDropdown = styled(Button)` | ||
| float: right; | ||
| margin-right: 3em; |
There was a problem hiding this comment.
When I see this margin-right: 3em in two places next two each other, it makes me think "should this be a constant?"
src/components/navbar/NavBar.tsx
Outdated
| src={ | ||
| 'https://lucys-love-bus-public.s3.us-east-2.amazonaws.com/LLB_2019_Sq_rgb+1.png' | ||
| } | ||
| alt={'Site Logo'} |
There was a problem hiding this comment.
alt="Site Logo" is how I prefer using strings.
| </Col> | ||
|
|
||
| <div> | ||
| {authenticated ? ( |
There was a problem hiding this comment.
| {authenticated ? ( | |
| { | |
| authenticated | |
| ? ( |
| </UserDropdown> | ||
| </Dropdown> | ||
| </Col> | ||
| ) : ( |
Why
We arent doing routing, antd styling, or styled components!
This PR
Adding craco antd, theming, and proper examples of styled components (replacing all instances of .less with equivalent styled component code). Also fixed routing issues where pages would be reloaded or history not pushed when navigating. Also added example navbar.
Verification Steps
Used npm run start and interacted with the site to ensure css was working as before and routing was not reloading the pages and was saving history to the browser.