Skip to content

Event link#49

Open
Lili00ani wants to merge 86 commits into
rocketacademy:mainfrom
Lili00ani:main
Open

Event link#49
Lili00ani wants to merge 86 commits into
rocketacademy:mainfrom
Lili00ani:main

Conversation

@Lili00ani
Copy link
Copy Markdown

No description provided.

Lili00ani and others added 30 commits March 22, 2024 08:56
added pages with router and navbar
Authentication Api integration Redux Register Login Logout
Comment thread README.md
@@ -1,5 +1,42 @@
# Rocket Academy Coding Bootcamp: Project 3 Frontend

# Event Link
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am missing here how to setup the environment variables to successfully run the project

Comment thread src/index.js
useRefreshTokens
cacheLocation="localstorage"
>
<BrowserRouter>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use two different routers here, when they both have the same setup?

Comment thread src/index.js
import AdminEventPage from "./pages/adminPages/adminEventPage.js";

const root = ReactDOM.createRoot(document.getElementById("root"));
root.render(<App />);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you delete the App.js? That is your entrypoint component usually

Comment on lines +55 to +59
useEffect(() => {
if (!isAuthenticated && !isLoading) {
navigate("/admin");
}
}, [isAuthenticated, isLoading, navigate]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exists pretty much in every admin component. Maybe we could use context to identify authentication status and re-use that instead of running the same useEffect in every component

Comment on lines +7 to +16
const formatDate = (string) => {
const date = new Date(string);
const options = {
day: "numeric",
month: "long",
hour: "2-digit",
minute: "2-digit",
};
return date.toLocaleDateString("en-US", options);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function could live outside of the component

Comment on lines +34 to +52
const formatDate = (string) => {
const date = new Date(string);
const options = {
day: "numeric",
month: "long",
hour: "2-digit",
minute: "2-digit",
};
return date.toLocaleDateString("en-US", options);
};

const formatHour = (string) => {
const date = new Date(string);
const options = {
hour: "2-digit",
minute: "2-digit",
};
return date.toLocaleTimeString("en-US", options);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find that you generally have a lot of opportunities missed out where you could have refactored code, and re-used certain functions and components. Try to not do things too manually all the time. This is bloating up your codebase and makes it harder to read also.

Comment on lines +126 to +128
if (loading) {
return <p>Loading...</p>;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you not do this everywhere you were just displaying a loading p tag?

Comment on lines +193 to +197
} else if (status === "open") {
return <Navigate to="/checkout" />;
} else {
return <p>Status: {status}</p>;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subjective, but i would place the one-liner conditions on top in separate if statements that return early. then place the huge chunk of code below those

gestureHandling={"greedy"}
disableDefaultUI={true}
>
{markers}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this variable can be undefined, as such you would not render any markers? Is that desired?

@@ -0,0 +1,7 @@
.div-no-event {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see a userPage component, so I assume this file is not named correctly? Especially since it is only used on the searchPage component.

import theme from "../../theme";
import Redirect from "../../components/Redirect.js";

function CustomTabPanel(props) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function CustomTabPanel(props) {
function CustomTabPanel({ children, value, index, ...other }) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants