-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
CYF Glasgow Dalila Boumezrag | Module-User-Focused-Data | week 2 #332
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cyf-ufd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Hi @DalilaBou,
Excellent work here with the form
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.
Hi @DalilaBou ,
I think it is better to use px for elements with fixed sizes (e.g border, and box-shadow). Hence, I will recommend using rem instead of px.
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.
Hi @DalilaBou ,
Brilliant work here. However, I have left a few comments for you to address. Happy to approve once this is addressed.
Thanks
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.
Hi @DalilaBou ,
See previous comment about using rem instead of px
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.
HI @mrvicthor thanks for your feedback i will go through it
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.
Hi @DalilaBou ,
I just observed that the alt attribute in the img element is empty. Was this intentional?
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.
HI @mrvicthor thanks for your feedback i will go through it
No description provided.