P
@sdkdeepaSubmitted 3 months ago
P
ComputerEnjoyer
@ComputerEnjoyerAll comments
- P@ComputerEnjoyerPosted 3 months ago
Very nice! Especially nice work in considering the tablet design.
Here are a few thoughts:
- It looks like you have two different stylings for the header section of each <article>. It is correct for Daniel Clifford, but incorrect for the rest. I believe this is because you are using a class "users" for Daniel and "person" for the rest, but only "users" is styled with a flexbox.
- The box shadow appears very turquoise. In general, the box shadow is often used as a very subtle effect to give elements a bit of depth, but it's very easy to overdo. The colors are also very tough to get just with your eye (without a design file). When in doubt, I would recommend going with #000 and then adjusting the opacity to get the right feeling.
Marked as helpful0 - @sbogeeSubmitted 3 months agoP@ComputerEnjoyerPosted 3 months ago
Hi Bogi,
nice job with this one!
A few thoughts for how you might improve this implementation:
- To change layouts, you're using a <table>, which is hidden in a narrow view but visible on large screens. This causes a lot of the HTML to need to be duplicated, which could make refactoring a pain. An alternative could be to change the layout using display: grid, which you adjust using media queries. In my view, this is actually the most straightforward way to achieve this layout. Checkout out Kevin Powell on YouTube if you want to learn more about grid! Otherwise, you can get started with this article from CSS Tricks.
- There is some buggy behavior with your media query at 1255px on my device. The layout changes, but it also bleeds over beyond the viewport width. My guess is that this is because there is nothing defining the max width for the desktop-display class.
- You're using 2 media queries, but really you only need one.
Marked as helpful0 - @AdienbobSubmitted 3 months agoP@ComputerEnjoyerPosted 3 months ago
Looks good! It looks like the font is not quite right in the button though. I think it's because the font-family is just never defined for button, so it is defaulting to serif.
Marked as helpful0 - @JeffTunnerSubmitted 3 months agoP@ComputerEnjoyerPosted 3 months ago
Nice job Jeff! You've done a really great job in implementing the tablet/desktop design.
A few things to consider regarding accessibility/best practice:
- What elements other than <divs> could you use to make it clearer what is inside the container? Here's a good intro on this topic from MDN: https://developer.mozilla.org/en-US/docs/Glossary/Semantics
- Consider using relative units (like rem) instead of px for text sizing. The reason you would want to do this is that the user might set a larger or smaller standard font size for their browser. Using relative units (like rem) respect those changes, and allows your typography to dynamically size based on the browser's standard text size.
1 - @tinohusseinSubmitted 3 months agoP@ComputerEnjoyerPosted 3 months ago
Very nice!
A few things that you might want to consider:
- You seem to be using different fonts than what is provided for this project. The project uses Google webfonts, which you can import from https://fonts.google.com/.
- The font weights are different between the solution and your design when it comes to the links and card__location.
- In a real-world scenario, I would consider wrapping each link in an <li> and making your card__social_link a <ul>. This will be more accessible for users with assistive devices like screen readers, which announce how many items are in a list as default behavior.
Nice job!
0 - @vanderloopSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
i'm proud of how i got the hover function to work. i would try to even out the li styling.
What challenges did you encounter, and how did you overcome them?trying to center the page was a struggle.
What specific areas of your project would you like help with?just basic constructive criticism!
P@ComputerEnjoyerPosted 4 months agoNice work!
A few things to think about:
- Your links should be anchors <a>, rather than just list items <li>. The reason why is that <a> changes the mouse behavior to indicate that something is a link. It also lets you add the actual href :) -The text inside the <li> is not centered vertically. You could try setting align-items: center in the <ul>. I think that would fix it.
- Instead of having a <div> of class "main-style", you could just use the <main> tag. All HTML documents should have a <main> tag for accessibility.
- On the same note as above, you could use a more semantically descriptive tag, like <article>, for your <div class="main">. It's not necessary, but is considered to be a more accessible practice.
0 - @hl484Submitted 4 months agoWhat are you most proud of, and what would you do differently next time?
N/A
What challenges did you encounter, and how did you overcome them?N/A
What specific areas of your project would you like help with?N/A
P@ComputerEnjoyerPosted 4 months agoNice work! I don't know XML so I can only comment on the implementation of the design.
Some feedback:
Your solution is not using the gap between elements that are defined in the design file. I find that one easy way to follow those guidelines is to try to align the auto-layout frames in Figma to containers in HTML.
The reason for this is that spacing between elements is almost never uniform. Items tend to be placed closer or further apart based on their conceptual relation to one another.
If you aren't already using Figma as a reference, I would definitely consider doing that! :)
0 - P@jgreen721Submitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Just a fun practice experience of reading in files and sending the data to the client. Using sockets, the client pings the server and the server establishes 2 readStreams (1 json file, 1 image) and sends the updates back upstream, as well as a filesize value so a progress bar can be rendered.
What challenges did you encounter, and how did you overcome them?Apparently FrontEndMentor has approved domains and it wasn't excited about the azure one so I set up a janky proxy static github that will re-route the user to the other site. Not ideal but seems workable enough solution for now. Any suggestions would be appreciated if there are better ways to do this.
What specific areas of your project would you like help with?Feedback and suggestions welcomed, especially for the stated above issue.
P@ComputerEnjoyerPosted 4 months agoHey! I don't think I can give feedback specifically on what you're looking for, but rather on an element of the visual presentation.
The qr code is getting squished on the righthand side because the progress bar still has a width and is still being displayed after the animation finishes. You can fix this by appending "display: none;" to the styles when the image loads :)
Marked as helpful0
