@SecretariatVSubmitted 4 months ago
Rachel Tschanz
@rtdevcraftAll comments
- @rtdevcraftPosted 4 months ago
Hi there! Nice solution!
Looks good on all sizes and nice separation of concerns! I like the UX of having the reset button with a lighter opacity until it is relevant.
Consider using semantic HTML like <header> and <main>
Looks like you have a typo for your aria-label here https://github.com/SecretariatV/FM-TIP-Calculator/blob/main/src/features/layout/tipPanel/index.tsx
You might search for 'ari' in other areas with CTRL + F (Command + F for OS) just in case you made the same typo elsewhere.
Not sure if starting with 0 people and an error as default is the best UX.
Looks like you could change the font to the space mono for the tip buttons and for the bull and number of people inputs.
0 - @simgeduruSubmitted 5 months agoWhat are you most proud of, and what would you do differently next time?
Even though the appearance closely resembles the design in Figma, I am not sure if the functionality is appropriate
What challenges did you encounter, and how did you overcome them?To make the design responsive, I initially gave each div height and width values in percentages to get the appropriate dimensions. I think this was the wrong approach. I resolved this by giving the first div inside the main the width and height values in pixels as seen in the design. At least, I think this is a solution. I am curious about how to solve such situations without using pixel values.
What specific areas of your project would you like help with?I know not everything in the project is suitable. Since I don't fully understand what's right and what's wrong, I struggle with asking questions. Therefore, I am open to learning anything you can suggest or see as missing. Do you have any tricks for creating a basic layout? For example, should the mobile design be done first, or should the common points between desktop and mobile designs be identified and built upon from the start? I would be happy if you could provide information!
@rtdevcraftPosted 5 months agoHey Simgeduru!
Just wanted to answer some of your questions and let you know it looks great!
Pixel values are fine as long as the it would be the same pixel value across all screen types, which in this case it is.
Doing the mobile size first is best practice, and then build up from there. As far as tips for building basic layouts, just depends on what you're building!
Marked as helpful1 - @moojicaSubmitted 7 months agoWhat are you most proud of, and what would you do differently next time?
Completing another challenge on FEM.
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
@rtdevcraftPosted 6 months agoHi there!
This looks so great!
I've heard that using tables isn't great for mobile-responsiveness, but it probably doesn't matter as much for a project like this when the design is so small.
Read more here: https://developer.mozilla.org/en-US/docs/Learn/HTML/Tables/Basics#when_should_you_not_use_html_tables
0 - @BhargavMaralaSubmitted 6 months agoWhat are you most proud of, and what would you do differently next time?
NA
What challenges did you encounter, and how did you overcome them?NA
What specific areas of your project would you like help with?Html and CSS
@rtdevcraftPosted 6 months agoHi there!
This is a close solution!
- Try out using more semantic HTML instead of just using divs
- Looks like the container could use a border-radius
- Get rid of the border on the buttons to more closely match the design
- Move the attribution below the container
- Delete commented out CSS code if you wont be using it
Marked as helpful0 - @chrisvn1808Submitted 7 months agoWhat are you most proud of, and what would you do differently next time?
I tried my best to make the solution has closest look to the design.
Add some simple animation, have a little fun with it.
What challenges did you encounter, and how did you overcome them?I have trouble selecting semantic HTML tags for category and publishdate element on the card.
I used tag in my solution for them to work. I might have used spans but I sticked to tag.
Not sure though if using tag in this context is good practices.
What specific areas of your project would you like help with?Any recommendations about the project would be valuable to me :)
@rtdevcraftPosted 6 months agoHeyo!
This looks great!
I only have a couple small things. For the alt tag, I think you can just put like 'abstract illustration in yellow and black' or something like that. Just try your best to describe the image, even if it's weird. It's just so people who are using screen readers get an idea of what the image is. I think when you are targeting with '*' that means everything, so you don't have to include *::before, and *::after, unless you're using those pseudo elements.
Great job!
0 - @R3lssSubmitted 6 months agoWhat are you most proud of, and what would you do differently next time?
Forcei na minha mente como centralizar uma div
What challenges did you encounter, and how did you overcome them?Não encontrei muitos desafios, apenas um erro de leitura meu que não consegui encontrar as cores certas
What specific areas of your project would you like help with?Acho que nenhuma
@rtdevcraftPosted 6 months agoHi there!
Unfortunately I cannot read the language you've wrote your answers in :( I hope you can read my answers in English. Good job including semantic HTML for the footer and making it responsive! Here are a few suggestions I have:
- Use the figma file to match the blue color of the background
- Import the google font from google fonts- use the style guide in the folder to help know which fonts and colors and such to use
- Put the footer below the main content
- Get rid of the empty span in your HTML
- Use padding and fix the text overflowing the main container
Marked as helpful0
