@AmneisaOB
Extendo99
@Extendo99All comments
- @Extendo99
Hey, great job.
Below I'm posting the mistakes I've noticed and the things you can fix:
- Unfortunately, the page only looks good on the computer. It is not adapted to tablet and mobile resolutions because elements can run away from the page on lower resolutions, e.g. 768px.
I recommend the mobile first approach, i.e. first write the code for the phone and then use media queries to expand and adapt the elements for higher resolutions. Here is a link explaining this approach: https://www.interaction-design.org/literature/topics/mobile-first
-
In my opinion, in this project, you do not need to use flex for every element. You can simply position block objects using paddings and margins.
-
Footer on desktop is not 100% wide.
-
Text has too wide containers. It looks different in the design.
-
Slight differences in margins.
Marked as helpful - P@wecaxWhat are you most proud of, and what would you do differently next time?
Starting to get the hang of CSS Grid
What specific areas of your project would you like help with?Any feedback is welcome
@Extendo99Hey,
Great job. The design looks almost identical. The code is readable and it's great that you use variables and rem units. It's great that you created a design for tablets so that the project will look good on various devices and not just phones and computers.
The only thing missing is a border around the profile pictures.
- @Saritha-Sasi@Extendo99
You should use a margin at the top of the page.
The tabs should be white on the solution.
The mobile view looks incorrect because it is not centered. The text in the paragraph also has no padding, which is why it touches the edge of the screen.
It's great that there is a tablet view implemented, consisting of 4 tabs in a rectangle :)
The code looks okay, but I would group all the rules for a given media query, e.g. 1024, in one place and media 768 in another, because there is currently some confusion. Once there is media for 1024, then for 768, and then without media, which makes it difficult to read.
- @justine1607@Extendo99
The style file should be called style.css
Add the font family that should be displayed if the user doesn't have it installed. In this case:
font-family: "Montserrat", sans-serif; font-family: "Fraunces", serif;
Overall it looks okay :)
- @nilmusWhat specific areas of your project would you like help with?
I gave up at adhering to the mobile design because I had no idea how to make the padding of only the top image disappear on smaller screens. Any clue is highly appreciated.
@Extendo99Hey,
The font colors are different than in the design. Yours are darker. The gaps between the rows in the table are too small. The margins/paddings between the elements are also slightly different, but overall the design looks very good. There are only slight differences in spacing and colors.
As for the mobile view, maybe it would be easier to start with it? The mobile first approach is that we start by coding the view for the phone and then gradually enlarge and adapt the elements to larger screens. I recommend it, I did it myself and had no problems with spacing or the photo.
- @ardaacikgozWhat are you most proud of, and what would you do differently next time?
I tried to use classes for manipulating CSS rather than using inline CSS.
What challenges did you encounter, and how did you overcome them?Locating the items to the center is hard thing to do. I used display block command and adding margin:0 auto command rather than display flex.
What specific areas of your project would you like help with?It would be great if you check my coding logic, responsiveness, and give feedback which parts I can improve.
@Extendo99The container has a different color than the one given in the project. There are also differences in the spacing between elements, especially the name and address. The font size also varies.
In CSS, you use enter for each attribute and there is a space. I recommend removing this to make the code more readable. It's better to use an external CSS file as well. Currently, the HTML file is difficult to read due to these CSS styles and it is difficult to make changes to it.
It is better to set the font size using REM
The code is not adapted to the different resolutions of mobile, tablet and desktop.
- @helmisulaemi@Extendo99
Everything looks fine and uses advanced methods.
- @rcsilva211What are you most proud of, and what would you do differently next time?
After a year working as a Salesforce developer, it was so refreshing to make something out of pure HTML and CSS without having to worry about limitations.
What challenges did you encounter, and how did you overcome them?Reviewing pure CSS attributes. In Salesforce and many other frameworks image implementation is automatically made in the majority of the time, so something as simple as making the image fit the container was a struggle, but after playing around for a bit, I've managed to make it work.
What specific areas of your project would you like help with?Best practices and code optimization
@Extendo99I will use the h1 tag in HTML because it is the main header in the project.
CSS styles should be in an external stylesheet.
The code is well-structured, in my opinion.
The solution is slightly smaller than the design.
