Conversation
…ed to overcome this problem)
…ing left: project comp, styling andd content: work and education comp
| <link rel="preconnect" href="https://fonts.googleapis.com" /> | ||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin /> | ||
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Rubik+Mono+One&family=Share+Tech+Mono&display=swap" | ||
| rel="stylesheet" | ||
| /> | ||
| <link rel="preconnect" href="https://fonts.googleapis.com"> | ||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> |
| <script | ||
| src="https://unpkg.com/react/umd/react.production.min.js" | ||
| crossorigin | ||
| ></script> | ||
|
|
||
| <script | ||
| src="https://unpkg.com/react-dom/umd/react-dom.production.min.js" | ||
| crossorigin | ||
| ></script> |
There was a problem hiding this comment.
I don't think you need to load react via a CDN script, if you are running this app on react already :)!
| render() { | ||
| return ( | ||
| <div className="App"> | ||
| <header className="App-header"> |
There was a problem hiding this comment.
I think this header component is redundant if everything is in the header :)! Should remove this, as it is semantically not correct
There was a problem hiding this comment.
Can also rewrite to main
| <header className="App-header"> | |
| <main className="App-header"> |
|
|
||
| import About from "./Components/About"; | ||
| import Education from "./Components/Education"; | ||
| import Menu from "./Components/Navbar"; |
There was a problem hiding this comment.
Let's call this Navbar, not Menu. Confusing
| import Menu from "./Components/Navbar"; | ||
| import Hero from "./Components/Hero"; | ||
| import Project from "./Components/Project"; | ||
| import Work from "./Components/WorkExpereince"; |
There was a problem hiding this comment.
| import Work from "./Components/WorkExpereince"; | |
| import WorkExperience from "./Components/WorkExperience"; |
| Portfolio (Frontend) | ||
| </Card.Title> | ||
| <span | ||
| style={{ color: "magenta", fontSize: "max(0.8vw, 28px)" }} |
There was a problem hiding this comment.
Have I mentioned I don't like inline styles? :D
| if (isMobile) { | ||
| element = <Mobile />; | ||
| } else { | ||
| element = <PC />; | ||
| } | ||
| return element; |
There was a problem hiding this comment.
by the way, this kind of pattern reads nicer like so, if you ever want to use it again:
| if (isMobile) { | |
| element = <Mobile />; | |
| } else { | |
| element = <PC />; | |
| } | |
| return element; | |
| return isMobile ? <Mobile /> : <PC /> |
| <Row style={{}}> | ||
| <Col sm={3} style={{}}> | ||
| <Nav variant="pills" className="flex-column"> | ||
| <Nav.Item> |
There was a problem hiding this comment.
use a map here, to make it less bothersome to write up all this repetitive code.
| const | ||
| expert = 85, | ||
| basics = 33, | ||
| intermediate = 50, | ||
| advanced = 70; |
There was a problem hiding this comment.
Interesting syntax, first time seeing this.
| <ProgressBar style={{ height: "2.3vh" }}> | ||
| <ProgressBar | ||
| data-aos="zoom-in-right" | ||
| now={advanced} | ||
| label={"ADVANCED"} | ||
| className="advanced" | ||
| style={{ fontSize: "1.4vh", color: "black", fontWeight: "bold" }} | ||
| /> | ||
| </ProgressBar> | ||
| <label | ||
| style={{ | ||
| fontSize: "max(1.5vw, 27px)", | ||
| fontWeight: "bold", | ||
| }} | ||
| > | ||
| HTML && CSS | ||
| </label> | ||
| <ProgressBar style={{ height: "2.3vh" }}> | ||
| <ProgressBar |
There was a problem hiding this comment.
Also very repetitive here, I think would be better to create some objects in an array detailing the dynamic datapoints and then using a map to generate the HTML.
|
@LoyChaiEe here your code review |
| <Nav.Item> | ||
| <Nav.Link | ||
| eventKey="first" | ||
| style={{ fontSize: "max(1.2vw, 20px)", fontWeight: "bold" }} | ||
| > | ||
| NTU(B.Eng) | ||
| </Nav.Link> | ||
| </Nav.Item> |
There was a problem hiding this comment.
| <Nav.Item> | |
| <Nav.Link | |
| eventKey="first" | |
| style={{ fontSize: "max(1.2vw, 20px)", fontWeight: "bold" }} | |
| > | |
| NTU(B.Eng) | |
| </Nav.Link> | |
| </Nav.Item> | |
| const tabItems = ["NTU(B.Eng)", ....] | |
| { tabItems.map((item, index) => ( | |
| <Nav.Item> | |
| <Nav.Link | |
| eventKey={index + item} | |
| style={{ fontSize: "max(1.2vw, 20px)", fontWeight: "bold" }} | |
| > | |
| {item} | |
| </Nav.Link> | |
| </Nav.Item> | |
| ))} |
| fontWeight: "bold", | ||
| }} | ||
| > | ||
| React |
There was a problem hiding this comment.
[{ label: "React", now: advanced }]
2 participants
No description provided.