archana-singh12• 150
@archana-singh12
Submitted
Mayssa Ghanmi
@MAY55A
All comments
- Mayssa Ghanmi• 120
@MAY55A
Posted
Amazing work, the solution matches the design, the code is well structured and readable. However, semantic html was not included. The layout looks good on most screen sizes but for widths between 375px and 530px, the layout isn't the best (some content isn't visible on the left).
Marked as helpful
0 - opiyo qaphael• 230
@Qaphael
Submitted
Mayssa Ghanmi• 120@MAY55A
Posted
Great work, clear nice code, responsive layout, and looks good on smaller screens. Only the code doesn't include semantic HTML, you can at least add a main, and I think the paragraph under the main heading (or what you called third-line) has a gray color not black.
0 - João Xavier• 230
@joaoxavier-profissional
Submitted
What are you most proud of, and what would you do differently next time?
Grid layout was a little akward to use, but i'm surelly that it'll make more sense in a few more days
What challenges did you encounter, and how did you overcome them?
Making a mobile first page was actually very challenging, trying to make it match with the desktop version after doing the style for the mobile was a fun challenge
What specific areas of your project would you like help with?
i would like some help to match the texts in the desktop version
Mayssa Ghanmi• 120@MAY55A
Posted
Good work in general, clean code and semantic HTML included. These improvements can be made :
- make the card size smaller for the desktop layout, reducing the empty spaces.
- use only one media query for the desktop layout, and make the mobile layout the default and therefore only change the different properties without writing every style of all elements for each design (the button and prices in both layouts are similar so there is no need to alter them in the desktop version and only write their style once in the default layout - which is the mobile one -).
- if you are having trouble with the grid layout in the desktop layout, you can use a flex display in the second column of the grid instead of dividing it into rows.
0 - Vutlhari Shiburi• 100
@Exterminator737
Submitted
Mayssa Ghanmi• 120@MAY55A
Posted
Good work, the solution generally matches the design, and the code looks clean and readable.
However, some improvements can be made :
- The divs should be changed to sections to make better use of semantic HTML elements.
- The h1 and h2 font sizes should be slightly bigger, have lower font weights, and be more responsive.
- The background color used in the solution isn't the one used in the design.
- For mobile screens, the image should fill the whole width of the screen meaning there is no padding around the image (look carefully at the mobile design).
0 - P
@Veena-K-Venugopal
Submitted
What are you most proud of, and what would you do differently next time?
I am proud of creating a solution similar to the preview even without exact design measurements.
What challenges did you encounter, and how did you overcome them?
Setting the style to highlight the list of links on hover was a bit challenging as the text color was still white even when I added the required color to the li:hover selector. I fixed this by adding color:inherit to li:hover a selector.
Mayssa Ghanmi• 120@MAY55A
Posted
Good work. Semantic HTML is included and the code is readable.
However, the dimensions and font sizes should be adjusted and resized based on the screen size.
Marked as helpful
0 - Gonzalo Anuar• 20
@gonzagapa
Submitted
What are you most proud of, and what would you do differently next time?
I like the way how fast I've finished the proyect.
I would like to try building it with tailwind css
What challenges did you encounter, and how did you overcome them?
I didn't know how to use and add custom fonts.
If you want to add custom fonts or self-hosted fonts, you only need to declare a font-fance rule.
Mayssa Ghanmi• 120@MAY55A
Posted
The solution seems to match in general, some changes need to be made to the weights of the fonts and make them resizable depending on the screen size.
Also, the HTML code isn't very semantic it requires at least a main tag, and thus, it is not very accessible.
The code is clear and readable though.
0 - Raul Galicia• 20
@raulgaliciab
Submitted
Mayssa Ghanmi• 120@MAY55A
Posted
The code is well-structured and readable, semantic HTML is also included, and the solution almost matches the design (I think there is a messing shadow effect around the card).
0
