@catherineisonline
szam
@k-stopczynskaAll comments
- @k-stopczynska
Hi! Great job on finishing the challenge!
Since you feel pretty good in CSS and HTML I have suggestions for you.
Firstly: try adding transitions on active elements like navbars/ hamburger buttons (it can be simple like opacity and scale, or moving from one side using translate X or Y with transitions ease-in/ ease-in-out).
When you feel comfortable with it, try some cubic-bezier effects: here is a playground for it: https://cubic-bezier.com.
After that try animations (like rotating some elements when clicked or sth like that).
When you do that and your projects will become more dynamic- it will be time for some preprocessor like Sass.
Hope this helps, happy codind!
Marked as helpful - @Leonardclc@k-stopczynska
Hi,
to answer your question: it's a good practice to start from mobile view and it won't be tyresome, it's less lines of code and fun:)
happy coding!
- @JudasThePriest@k-stopczynska
Hi!
Congratulations on finishing the challenge!
You did good job with JS part of validation but please consider that if you share your code on a forum it needs to be cleaned up. Especially if you use such colorful console.logs: DDDD you wouldn't want to share this with someone who would want to hire you.
I would also try not to style anything through JS. It may cause problems in the future because JS styles overwrite CSS styles and it's hard to find in bigger projects. In JS add some class and style this class in CSS.
I would also try to improve responsiveness and get some layout for tablet version and screens bigger than 1440px.
Also fix links in documentation. I know it's a small project but it's a good habit to have (still working on it: D).
Overall good job, you overcome the problems, learn how to search for solutions and get very close to the design:)
Happy coding!
Marked as helpful - @Lukasz-Milde@k-stopczynska
Hi!
Congratulations on finishing this challenge!
One question from user's point of view: why is there a default score? Wouldn't it be better not to submit at all and get some error message if the user haven't had picked any score?
I reviewed your repo and must admit: very nice and clean code, good documentation and close to design: )
Keep up the good work and happy coding!
Marked as helpful - @AhmedAbdulatif@k-stopczynska
Hi!
congratulations on completing the challenge!
It sure looks good although could be centered as in design. Nice job with theme switcher.
Just tested the live version and there are 4 things that shouldn't be happening:
--if you subtract by 0 you should get an error not Infinity --if a user type 18 signs or more- it starts to overflow in X axis -- if a user try to get even 6 decimal digits and subtract it by another decimal- it starts to overflow in Y axis -- there is a problem with big numbers (I didn't get the result as in my calculator) And from user point of view- calculator should have an option to use with keybord.
Hope this helps:) Happy coding!
Marked as helpful - @codedilla@k-stopczynska
Hi! Congratulations on completing this challenge! Looks nice!
I've reviewed your code and have few pointers to improve it in the future:
Remove predefined styles from HTML file. If you use something like styles.css you should keep all the styles in this place. HTML is for markup, css for styling, JS for user experience.
I don't really get why but every markup element is in it's own div. It's bad for accessibility. These tags are really important for search engines and screen readers. Also keep track of your headings, don't fit them to design (like you did with h6)- you can always change fontsize, fit them to their purpose. Also 'Mark all as read' is actually a button.
You left empty classess in few places. It's easier to read code without blanks.
Try not to mix selectors in css, start from html tags and get deeper with specificity. or start from the top of document and get down, but try to maintain one method everywhere.
There are also some conflicting styles like: all the headings get font-size .9rem (probably not the best practice either) and few lines after that class heading (which is on h1) gets font-size 1.3rem.
Practice naming your functions and classes. Should be very descriptive like: function markAsReadHandler instead of read, class spandiel is hard to understand too.
There is a place for a link to your FrontendMentor profile in attribution:)
Good job with responsiveness and making it very close to design!
Hope this helps, happy coding!
Marked as helpful - @giandefaria@k-stopczynska
Hi!
Congratulations on finishing the challenge!
I have two suggestions based on testing live version:
-
You did fix the outcome but didn't prevent the user from inputting values like: 123,567789 and they do appear as a tip value as user is typing and overflowing the container.
-
I can easily set No. of people and bill amount to negative numbers as well as get custom negative tip, you probably don't want that (simple min value on the input field would do the trick).
Overall nice job, happy coding!
Marked as helpful -
- @browynlouis@k-stopczynska
Hi!
congratulations on finishing this challenge:) Looks really nice!
Few things you might find helpful: I would probably split this into two different components in React in order to get more reusable code and more logic to the project.
You are using Sass but you don't use mixins (there is a mixin file but it's empty). It's super helpfull to use them e.g. for media queries. Also variables can by anything you want, not just colors, you can get borders, border-radius, fonts, font-weights in there etc. That way you have more reusable code and make changes only in variables file. It is good practice to use specific file structure in Sass, this article will help you start: https://itnext.io/structuring-your-sass-projects-c8d41fa55ed4
Happy coding!
Marked as helpful - @Basim1998@k-stopczynska
Hi,
very nice and clean solution:) congratulations!
One thing you might wanna improve: before submission check if a user clicked on any rating. Now you can pass an empty submission and get a thank you card.
Happy coding!
- @FrancoMinati@k-stopczynska
Hi,
your solution looks really good, congratulations:)
Few things I might suggest: Use array methods instead of loops, it's more readable, faster and convenient (e.g. looping through buttons, you could use forEach method which is available for HTML collections and Node lists).
For Sass: it's a little overkill using Sass for project so small, but when you are using it: look at your modules structure. Here you can find tips how to structure it: https://itnext.io/structuring-your-sass-projects-c8d41fa55ed4. Kevin Powell has also youtube video on this topic.
And one another thing: with new version of Dart Sass it's not really recommended to use @import. Kevin also has a video on using @use and @forward, it's also available in Sass documentation.
Hope this helps:)
Happy coding!
Marked as helpful - @romila2003@k-stopczynska
Hi,
Congratulations on completing the challenge:)
I don't know if you noticed but the data isn't loaded until you click the button. It would be much better if you selected e.g. weekly stats as default.
Very nice job with this one, code is clean, I can easily figure out what you are aiming for. Happy coding!
Marked as helpful - @k-stopczynska@k-stopczynska
Hi,
Yeah, forgot to fix decimals and also about the error message.
Thanks for review, really appreciate it.
Happy coding!
- @PhilipKorb@k-stopczynska
Now, that's better:)
You're fast learner for sure, just keep coding and you'll get there faster than you think:)
Marked as helpful - @Aur71@k-stopczynska
Hi!
According to your question: select them by const buttons = document.querySelectorAll('.buttons-class'). You will get the Node-list of them. Add event listener to all of them and write a function which on the click:
- removes class active from all the buttons
- on the button that is an event target will add class active
- will provide data for your span dynamically, e. g
const spanValue = e.target.innerContent;
and then you can use it like this
${spanValue}
You can also check my solution with different approach. Hope this helps:)
You did very good on the layout!
Happy coding!
Marked as helpful - @selly361@k-stopczynska
Hi!
congratulations on completing the challenge:) Looks really good, but you could improve it by simple modifications:
For mobile devices set a min-height: 100vh to the body, that would simplify your code and prevent cutting the container right off.
I see you set the border-radius for mobile, but you chose the wrong element (divs first child is an image). Add some id to your divs in main, it would be easier to point at them this way. Or use main: first child and it would point to the right element.
On hover states on buttons, as far as I remember, shouldn't be any border set. So, either you set it to none, or keep the border the same color as background or transparent.
I don't know if you have noticed but your sedans section always starts higher than others in desktops. That's because it's longer and you set margin-bottom the same for every section. Maybe it would be better to fix it just using margin-top.
Are you really certain you want attribution on top on mobile?
Happy coding!
- @y25sanchez@k-stopczynska
Hi!
congratulations on completing the challenge, looks really great:)
One thing I've noticed: on desktop your Karma and Calculator sections are not really in their designed places. You could easily fix it using grid instead of flex. Then you can point exact grid-area on any element.
Happy coding!
Marked as helpful - @VenusY@k-stopczynska
Hi!
Well, there is at least one reason I would think of: regex doesn't check if there is such a domain or if this email is actually yours (so security reasons, probably some tokens would be in play for this). Usually in regex you also don't check for addresses with IP in it.
But for email validation like this newsletter- I would say it's enough for filtering if email is valid. For logging purposes- definitely not but I never dived into it that much at this point of my learning.
Hope this helps:)
- @VenusY@k-stopczynska
Hi!
For email validation- just use regex. Your solution unfortunately got me submit successfully the email: %^%&^*@gmail.com and kstop@$%^&%$&$.com. As you know they are not correct email adresses. As for the media-queries in JS- when you really need to- use them. But mostly styling should be done in css. You could use JS to add a class for certain elements and just style this class. I used to do the same thing:)
Congratulations on finishing the challenge! You really did well on matching the design:) Keep on coding!
Marked as helpful
