Skip to content

Enhance Profile Page - #15

Open
iamalexov wants to merge 7 commits into
devfrom
feature/profile-enhancements
Open

Enhance Profile Page#15
iamalexov wants to merge 7 commits into
devfrom
feature/profile-enhancements

Conversation

@iamalexov

Copy link
Copy Markdown
Collaborator

Implemented the Profile page with:

  • Full Name
  • About Me
  • Programming Languages
  • Soft Skills
  • Sports
  • Hobbies

Added form validation and responsive styling.

Backend integration will be implemented in a future PR.

@nicostuhlfauth nicostuhlfauth left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @iamalexov 👋
Thanks for requesting the review from me. I left you some comments inline, but more generic, there is an issue how you use CSS and structure your code. To improve:

  • do not use any global CSS in the component CSS
  • use CSS modules to make sure that the CSS only has an impact on the component it belongs to. See this quick tutorial.
  • Extract components, that's what React is about. For example create an Input and a Button component, each with its own styles that can be reused on the pages. This keeps consistency. Another option is to move your Container into a component, so you can have the same margin and padding everywhere.
  • Define your font sizes in a central place, not per component. You can also use components for that or, at least, have this one in your main css file.

Happy to sync this evening about it if needed.

Comment on lines +1 to +10
* {
margin: 0;
padding: 0;
box-sizing: border-box;
}

body {
background: #f3f6fb;
font-family: Arial, Helvetica, sans-serif;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These styles should not be defined here.


.container {
width: 100%;
max-width: 100%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You already set the width to 100%, so you can remove it.

Suggested change
max-width: 100%;

Comment on lines +34 to +49
input[type="text"],
textarea {
width: 100%;
padding: 14px;
margin-bottom: 20px;
border: 2px solid #ddd;
border-radius: 10px;
font-size: 16px;
transition: .3s;
}

input[type="text"]:focus,
textarea:focus {
outline: none;
border-color: #2563eb;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just as an example: These styles do not only change the styles of your input on the Profile page, but also on other pages, where any input is used.

@@ -0,0 +1,9 @@
import styles from "./Container.module.css";

export default function Container({ children, className = "" }) {

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
export default function Container({ children, className = "" }) {
export default function Container({ children }) {


export default function Container({ children, className = "" }) {
return (
<div className={`${styles.container} ${className}`}>

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
<div className={`${styles.container} ${className}`}>
<div className={`${styles.container}`}>

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.

2 participants