@Rahulbaran
@romanrobert
Submitted
Hello 🖖,
Started learning React, and this was a really nice project to solidify what I learned so far.
What I noticed is that the advice is cached for 2 seconds and any repeated request within 2 seconds will return the same piece of advice.
Feedback is always welcomed.
@Rahulbaran
Posted
Good evening/morning Robert, I am Rahul from Bharat(India). Here is my feedback:-
Use heading tag for large font-sizes (wrap quote in h1 tag instead of p).
CSS Custom properties are very handy and make your code maintainable. Learn to use them (An article by Smashing Magazine, if you are interesed to read)
Wrap dice image inside a <button> tag instead of <span>, since it is a button and you should keep accessiblity in mind.
Always use catch() along with then() to handle an error while making the request.
No need to import React in App.jsx.
Marked as helpful
@wtwilliams310
Submitted
All feedback is welcome.
@Rahulbaran
Posted
Hi Williams, I am Rahul.
I went through your code and here is my feedback:-
Your button is not rounded so instead of using border-radius: 50px, use border-radius: 50%
You have mentioned two font-families in body element. since CSS cascades, there will be not effect of first one, so you can remove one of them.
You can use custom properties for other properties like font-size, font-weight, padding, margin etc too.
You should provide a smaller width for mobile devices by using min() CSS function.
@adil-khursheed
Submitted
Fetching data from an API is one of the most satisfying job for a developer.
Any feedback is most welcome!!
@Rahulbaran
Posted
Hi Adil
You can furthur make your code more maintainable by using custom properties for font-sizes, padding, margin, font-weight, line-height etc and calculating the required value with css "calc()" function.
Marked as helpful
@tifflee7784
Submitted
all feedback are welcome :)
@Rahulbaran
Posted
Hi, I am Rahul. I went through your code and here are the things I found:-
By default, a browser applies some paddings and margins which makes it tough to place the items properly, You can resolve this by setting margin and padding to zero in universal selector.
Progress bar is not positioned correctly,
Use "rem" or "em" instead of "px" for font-size, padding & margin
background image for desktop version is not positioned correctly
Marked as helpful
@varisDogukan
Submitted
Mobile size 320px.
@Rahulbaran
Posted
Hi, I am Rahul. You should increase the inline-padding of the expense card.
@Sg108
Submitted
feel free to comment
@Rahulbaran
Posted
Hi Shivam,👋 I am Rahul.
Some points which I am written here after watching your code:-
You don't need to copy the data from data.json. You should learn about XMLHttpRequest API or Fetch API to handle the json data.
Instead of using querySelector method to select html elements(tags) individually use querySelectorAll method, Read here for more info. You can also checkout my code about the same project.
Use rem or em instead of px to get more controls on over your site responsiveness.
That's all from my side, Happy Coding🙂👨💻
Marked as helpful
@CaioRoman
Submitted
I couldn't change the color of the image, could someone teach me how to do this or where I think ???
p.s. sorry for my english, i'm brazilian and i'm still learning english so i'm using google translator
@Rahulbaran
Posted
👋Olá Roman, I am Rahul.
I went through your code and here are some points which I want you to know regarding the site
You should learn the flexbox & grid layout to make the site responsive.
Instead of "img" tag , you can use the image as background along with the background color which you saw in the preview of project and you can go through the mdn link.
Use rem or em instead of px , it will give your more control to change the font-size & other unit related properties. You can read about it here.
Learn some shorthand properties like margin, padding, font etc. to make your code shorter and cleaner.
Here you don't have to use two stylesheets (css files).
That's all for now hope it helps and let me know if you need to know about some resources etc.
Thank you for teaching me how Someone greet in brazilian. Happy Coding🙂
@ijyotimaurya150
Submitted
Hi! I tried my best to replicate according to the given task, but I cannot correct those four cards correctly. Help me with alignments!
@Rahulbaran
Posted
Hi Jyoti , I am Rahul. I think you have just started web development, there is no need to panic this is going to take time to understand many of the properties in CSS but that's okay with time you will see improvement in your code and will be able to understand all these properties , just keep reading articles, watch some videos in youtube, join some web development communities like available in slack or discord and keep asking questions. One thing I would like to focus on that whenever you have some issues related with your code then do google search since you already know it's the best place for getting answers.
Now let's come to the issues I found in your code , there are many but I would point just the important ones:-
i. Read about margin and padding here :-https://www.thoughtco.com/css-zero-out-margins-3464247 ii. Read about universal selector here :- https://www.geeksforgeeks.org/what-is-the-use-of-asterisk-selector-in-css/ iii. Read about box model and box-sizing here https://www.udacity.com/blog/2021/04/the-css-box-model-explained-for-beginners.html?utm_source=rss&utm_medium=rss&utm_campaign=the-css-box-model-explained-for-beginners
Do google search for more details about it .
You need to grasp the concepts of grid system to align these cards properly , it's a huge topic and you will have to spend a lot of time to understand this. Read here about it https://css-tricks.com/snippets/css/complete-guide-grid/ You can download book based on grid system through the link I have provided below:- https://www.pdfdrive.com/grid-layout-in-css-interface-layout-for-the-web-e176071540.html (Here you can check out for other books if you want).
No need to use transform property , applying grid system will do the job .
I know there is a lot to read and also practice but these resources are definitely gonna be beneficial. here I have suggested some websites which will be quite helpful in your web development journey :-
https://css-tricks.com/ (it is a great site for css and there are great articles here.) https://stackoverflow.com/(it had answers for your 99% questions on web development) https://developer.mozilla.org/en-US/docs/Web/
That's all from my side, hope you become a great developer. Take Care and let me know you have something to ask.
@aasthaanand123
Submitted
I would really appreciate if you guys can give feedback about to how my code is. Thanks!
@Rahulbaran
Posted
Hey Aastha , Hi I am Rahul. There are a number of issues towards which I would like to highlight and sorry about such a big list of issues . I have found following issues and added some links for help:-
Instead of using button tag inside your form tag use input type="submit" , which will make your code more accessible. Also there are issues with other input fields and labels. you should use field specific input types instead of using type="text" for each field. For example:- password field ----> type ="password" submit field ----> type="submit" email field ----> type="email" This is a great article by CSS-Tricks (it is an amazing site for web development related things) which has everything I mentioned above along with some extra amazing stuffs :- (https://css-tricks.com/html-inputs-and-labels-a-love-story/) Now let's come to the second issue
While adding attribute target="_blank" in anchor tag, you should also mention rel = "noreferrer noopener" . Read about it here:- Link-1 : -- (https://stackoverflow.com/questions/50709625/link-with-target-blank- and- rel- noopener-noreferrer-still-vulnerable) Link-1 :-- (https://web.dev/external-anchors-use-rel-noopener/)
While using id attribute when you have to mention one id with more than character then you can use either dash(-) or underscore(_) . adding space will create more than one id. Eg:- <h1 id="First Name"></h1> // two id's "First" and "Name" // instead use this <h1 id="First_Name"></h1> // one id First__Name(First-Name can also be used)
Here are some suggestions which I would like to deliver:-
That's all from my side. I hope, next time you write more accessible code Read here about accessibility:- (https://www.w3.org/WAI/standards-guidelines/wcag/).
Take Care and be consistent.
@JeremyMen
Submitted
Do you think that my style.css file is well organized? Thanks for your feedbacks
@Rahulbaran
Posted
Hey Jeremy , I know you tried to make it similar as shown in the picture. But you need to work on certain things:-
Also you should learn about rem and em units & use them in padding, font-size etc.
I know there is a lot to research but you already know , doing research will make you a better developer.
