@DavidDelannoyDeveloppementSubmitted over 2 years ago
Tic_Tac1602
@TicTac1602All comments
- @TicTac1602Posted over 2 years ago
Bonjour, ce site est entièrement en anglais pour tes prochaines proposition de solution merci de tout rédigé en anglais (le contenu de ta solution et tes noms de classes etc) comme ca toute la communauté pourra t'aider, pas seulement les français.
Tout d'abord bien joué le rendu final à la bonne tête cependant regarder bien la différence entre ton design et ta solution. On peux notamment voir que qu as un encadré au milieu qui n'as pas sa place ici. et du coup je suppose que ton element QR-code étant à l'intérieur il se retrouve plus petit que ce qui il devrait être, même si on est d'accord il ressemble bien a ce que on avait demandé.
Pour ce qui est du code en lui même sur la page de ta solution il y a plusieurs problèmes ACCESSIBILITY que tu peux résoudre assez simplement. Et pour ton HTML pareil sur la page de ta solution le site te proprose des choses pour l'améliorer. Je te laisse y remédier.
Encore une fois bon travail tu y est presque. Et n'oublie pas tout est en anglais ici
TicTac1602
Marked as helpful0 - @alicee88Submitted over 2 years ago
Hello, this is my first Frontend Mentor challenge and I'm very new to HTML/CSS so would appreciate any feedback on the following:
- I'm not completely sure what should be an id and what should be a class, is my usage here ok?
- I found naming tough and felt a bit conflicted about naming elements in the html to reference in the CSS (should I just reference the elements by attribute instead of giving everything a class/id?)
- Layout of CSS - what can I do to make it more readable/clearer?
@TicTac1602Posted over 2 years agoHello Alice, first of all nice job everything works just fine. I will try to answer your question by putting my response after the question.
"I'm not completely sure what should be an id and what should be a class, is my usage here ok?"
An id is unique where classes can be shared between different component. You made a good use of them since you have used only ID. And it is correct since almost every part is working by its own and do not share styling with another component. Nonetheless, you could have used a class for both cards where you could have set the background color the border-radius and so on... As the 2 cards are sharing those styling.
"I found naming tougher and felt a bit conflicted about naming the elements in the html to reference in the CSS (should I just reference the elements by attribute instead of giving everything a class/id?)"
Giving a class and/or an ID is nice for accessibility Reason. So that person can know what they are pointing or looking or hearing. So naming is up to you, but the BEM method could help you make naming easier.
Layout of CSS - what can I do to make it more readable/clearer?
I found the layout pretty much noise. But something made me ask questions. To what extend is the rating score a button? To what extend is the star icon a button? For me this should not have been a button HTML tag
Those are the fix I can see to your question, please let me know if it helped you getting even better
Once again very nice job on this challenge TicTac1602
Marked as helpful0 - @GloriaOkekeSubmitted over 2 years ago
All feedbacks are welcome...
How is my css styling? I will appreciate if there are better practices
@TicTac1602Posted over 2 years agoHello Gloria, from what I can see the deployment of your solution didn't go well.
I took time to download your repo and check your work. From what I can see on this one here some an advice:
-The font used is not the right one, in fact you didn't even import one, it's the default style of your browser. To fix that please refer to your "style-guide. med" file at the bottom you will find the name, font-size, font-weight you need for the project
-For the mobile version try to work with media queries. It's made it a lot simpler to understand what you did and the liability of the code will be better. (Even thought there is not much to read, but imagine on bigger projects).
Any ways very nice attempt keep on the good work.
Tic_Tac
Marked as helpful0 - @thecoltanreedSubmitted over 2 years ago
Resubmitting this because I realized in my first attempt, I misunderstood some of the calculations. I was getting the tip amount and diving only that amongst the people. This time, it gets the tip amount and divides it amongst everyone, and it also adds the total tip to the total bill, dividing that equally amongst everyone as well. Hopefully I got it right this time.
@TicTac1602Posted over 2 years agoHello, very nice work!
Here some quick fixes:
-Make the calculations update each time we modify a field in the form. To illustrate that you can test by entering 231 10% for 1. Give you a nice look. But if change 1 to 2 no update on the screen, even thought, every information is correct and possible to make the calculations.
-Highlight the reset button in light green when you have entered something. For now, until whoever it feels like a disable state.
Hope I've helped you improve your solution. And again great work :D Best regards
1 - @AbbasvoraSubmitted over 2 years ago
I would like to know how to minimize code duplication in CSS.
@TicTac1602Posted over 2 years agoHello! First of all, very nice job on the desktop version, it's pixel perfect. The difference that we can see in the "Design Comparison" is just because the attribution is on.
To answer your question on how to have less CSS duplication, you are using in this project ": nth-of-type()" which is nice when you want to tell a specific rule to a specific child. It is a nice use of this rule that you made, but some of them could be donc another way.
For example, all of your hover buttons have some white color and a pointer and a background color the same as the card so instead of :
.column:nth-of-type(1) button:hover{ color: hsl(0, 0%, 95%); background-color: hsl(31, 77%, 52%); cursor: pointer; }
.column:nth-of-type(2) button:hover{ color: hsl(0, 0%, 95%); background-color: hsl(184, 100%, 22%); cursor: pointer; }
.column:nth-of-type(3) button:hover{ color: hsl(0, 0%, 95%); background-color: hsl(179, 100%, 13%); cursor: pointer; }
You could have done :
.column button:hover{ cursor: pointer; color: hsl(0, 0%, 95%); background-color: transparent; }
Normally you are not losing any style that you already had but you have less CSS to write.
Hope this help you in this project and for the next ones.
Best regards
Marked as helpful1 - @tmorisSubmitted over 2 years ago
Another challenge completed, am requesting for any guiding and advise, Thanks
@TicTac1602Posted over 2 years agoHello, very nice work, here's some advices to make it even better:
First, imagine the full width of the desktop version as 5 parts (space, block1, block2, block3, space). By quick maths you know that your container muss do 60% of your body. You can even use 60vw. If you don't know this unit it is a percentage based unit scale based on the viewport the site is displayed on.
Then you could add a bit of padding to your cards, this will shrink naturally the space for the text and make your card taller (as the design).
Finally, you could try to import the correct font used in the tittle of the cards that you can normally find in your "style-guide.md" file.
Again, very nice work!
Marked as helpful1 - @rahiii-devSubmitted over 2 years ago@TicTac1602Posted over 2 years ago
Hello very nice work on the desktop version but maybe you should look on the mobile version. In your css media query for under 800px you set
*:hover{ all:unset; }
Wich make basicly your page loose all of it's styling (unless rewriten after this line). In fact by putting my cursor on the screen allmost all of your nice CSS disapear.
maybe you should find another way to do what you expected to do.
Cheer nice work anyway !
0
