-
Notifications
You must be signed in to change notification settings - Fork 1
Add dummy change to test gemini integration #8
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| import React from "react"; | ||
|
|
||
| export const LoginButton: React.FC = () => { | ||
| const login = () => { | ||
| const login = React.useCallback(() => { | ||
| window.location.href = "http://localhost:4000/auth/login"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoding URLs makes the application difficult to configure for different environments (e.g., development, staging, production) and is a maintainability concern. It's best practice to externalize configuration values like this. Consider moving this URL to an environment variable. For example, in a Create React App project, you could use |
||
| }; | ||
| }, []); | ||
|
Comment on lines
+4
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The use of If performance optimization is the goal, you should also wrap the component in |
||
|
|
||
| return ( | ||
| <button type="button" onClick={login}>Login with Google</button> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,12 @@ | ||
| import React from "react"; | ||
|
|
||
| type Props = { | ||
| name: string; | ||
| name?: string; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making the To improve this, you should consider either conditionally rendering the element or providing a fallback value. Example (conditional rendering): {name && <h3>{name}</h3>}Example (fallback value): <h3>{name || 'Anonymous'}</h3> |
||
| picture: string; | ||
| }; | ||
| export const Profile: React.FC<Props> = ({ name, picture }) => { | ||
| return ( | ||
| <div> | ||
| <div style={{display: "flex", flexDirection: "column"}}> | ||
|
Check failure on line 9 in client/src/components/Profile/Profile.tsx
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inline styles are best avoided for maintainability. They mix styling with logic, are hard to override, and don't support features like pseudo-selectors or media queries. Please extract these styles into a CSS file and use a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using inline styles is generally discouraged because it mixes styling concerns with component logic, making styles harder to reuse, override, and maintain. It's better to use CSS classes with a separate stylesheet for better separation of concerns and maintainability. For example: In <div className="profile-container">In a corresponding CSS file: .profile-container {
display: flex;
flex-direction: column;
} |
||
| <h3>{name}</h3> | ||
| <img src={picture} alt="Profile picture" /> | ||
| </div> | ||
|
|
||
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.
This hardcoded URL makes it difficult to deploy the application across different environments (development, staging, production). It's recommended to externalize this configuration using environment variables. For instance, you could define
REACT_APP_API_URLin a.envfile and construct the URL from it.