-
Notifications
You must be signed in to change notification settings - Fork 0
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
Wishlist, login and signup #8
base: Main
Are you sure you want to change the base?
Conversation
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.
Please check the indentation of your file. Rest is fine. good to merge.
} | ||
/* .login-check:valid { | ||
border: var(--Mak-colors-green-500) solid 1px; | ||
border-radius: 1.5rem; |
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.
remove commented code
<span className="avatar-img-container"> | ||
<img | ||
className="avatar-img" | ||
src="../images/steve.jpg" |
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.
src might not work here
</p> | ||
|
||
{WishlistProducts.map( | ||
(product) => { |
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.
You can destructure product values in this line
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.
Great job.
Things I liked -
- use of pattern attribute in input
- good folder structure and naming
Things where you can improve - Please give values to blank attributes
className="login-check formInput AlertWarning search-container" | ||
type="email" | ||
name="" | ||
id="" |
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.
Give values to these attributes.
Suggestion - use label for inputs
/* .login-check:valid { | ||
border: var(--Mak-colors-green-500) solid 1px; | ||
border-radius: 1.5rem; | ||
} */ |
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.
Please remove commented code
export function Login() { | ||
return ( | ||
<> | ||
<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.
Here you can remove these fragments as you are already enclosing your code inside a div
name="" | ||
id="" | ||
pattern="(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{8,}" | ||
title="Must contain at least one number and one uppercase and lowercase letter, and at least 8 or more characters" |
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.
appreciation - use of pattern attributes for regex
name="" | ||
id="" |
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.
Here also please give these attributes some values
alt="" | ||
srcset="" |
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.
give alt and srcset some values
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.
Good work. Code is clean. Just few suggestion mentioned. You may have a look at.
<div className="inputDomMainContainer flex-column-center"> | ||
<form | ||
className="formContainerMain inputDomContainer AlertInfo input-flex inputError box-shadow" | ||
action="submit" | ||
autocomplete="on" | ||
> | ||
<div className="formContainer input-flex flex"> | ||
<h3 className="h3Tag"> | ||
Forgot your password ? | ||
</h3> | ||
<p className="discriptionText"> | ||
Please enter the email you use | ||
to sign in to Vision Cart |
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.
Indentation part can be fixed in overall code
/* .login-check:valid { | ||
border: var(--Mak-colors-green-500) solid 1px; | ||
border-radius: 1.5rem; |
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.
Commented code can be removed if not required.
<input | ||
className="login-check formInput AlertSuccess search-container" | ||
type="password" | ||
name="" | ||
id="" | ||
pattern="(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{8,}" | ||
title="Must contain at least one number and one uppercase and lowercase letter, and at least 8 or more characters" |
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 is new to me, that using pattern attribute for input. Appreciate it !!
key={product.id} | ||
> | ||
<div className="card-container-horizontal flex-column-start"> | ||
<section className="cart-product flex-column-center"> | ||
<section className="flex-row-start"> | ||
<img | ||
className="imgcard" | ||
src={product.img} | ||
alt="product-img" | ||
/> | ||
</section> | ||
</section> | ||
</div> | ||
|
||
<span className="card-text-container cart-brand-text"> | ||
<span className="card-description"> | ||
<h5>{product.name}</h5> |
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.
For usability, "product" can be destructured. It's a suggestion only.
Adding Wishlist, login and signup