@ryanbradley-webdev
@AnisBacha
Submitted
I was unable to find a straightforward method to change the background color of the selected button, so I resorted to using a lengthy JavaScript code. While it may not be the optimal solution, it does effectively achieve the desired result, any better way to achieve better results, please tell me.
@ryanbradley-webdev
Posted
Great job! I went through your code and have a suggestion for optimizing the button states.
Using radio buttons is a great idea for this because it automatically unchecks a selected button if you select a different one. You can actually target the buttons using CSS with basically no JavaScript. If you use a CSS selector for "input[type="radio"]", you can target both the ":checked" and ":hover" states, setting the appropriate style for each.
I hope this helps! Great job again!
Marked as helpful
@felix-toledo
Submitted
Any feedback is good for me!
@ryanbradley-webdev
Posted
Great work on this project!
I viewed this on two separate browsers, and the SVG appeared on both with no errors. I also went through your code and didn't see anything that would cause this error after deploying. I'm assuming the issue arose as a result of the way Vercel deploys applications, possibly the way they build and deploy the app and then update build cache afterwards. Hopefully it's just the result of a bit of lag time with uploading images.
A way you can avoid this with SVGs in Vite is to make dedicated React components whose sole purpose is to return the SVG:
export default function MySvg() {
return (
<svg>
<path ... />
</svg>
)
}
I prefer to do this or just place the SVG directly into the component vice using an 'img' element to avoid import issues like this, and also to allow custom styling if I need it. It also makes reusing SVGs in application super easy!
Either way, great job! I hope this helps, and happy coding!
Marked as helpful
@AzizFezz
Submitted
I'm not sure about my css file and don't know if its responsive
@ryanbradley-webdev
Posted
Great work!
At first glance it seems responsive. I looked through your CSS file and it seems pretty good, although there are a couple instances of using flex/grid properties when the display isn't set to either. Not a huge issue, but I could lead to some frustration.
The one thing I noticed when looking at it on a smaller screen is that the footer overlaps the main content. Positioning the footer as absolute can definitely cause this if there's any overflow. An easy fix I've found when the page content is minimal (like this project) is to set the body's display to flex, direction column, min-height 100vh, and then set the main element (or whatever element you want to take up the most space) flex-grow to 1. This should shove everything else to the top or bottom as appropriate.
I hope this helps! Great job on this project!
Marked as helpful
@JS-Law
Submitted
This one took a lot out of me. There were many things I would do differently going forward. First being that I will not mix responsive sizing measurements with absolute measurements. That alone made this a nightmare to finish but regardless, I'm proud of what I brought to the table and I'm even prouder of what I'll take away from it.
The next project will be approached differently from the ground up.
@ryanbradley-webdev
Posted
You should definitely be proud of this, it looks great! Only a couple things I noticed, first is the use of semantic HTML. The use of "main" and "footer" elements will make your application more accessible and therefore better performing for search engines. That's obviously not a concern for this project, but it'll help in the future, plus you can cut back on applying classes to containers and make the code a bit more readable, although your use of comments helps a lot!
Second, I ran through your code and noticed a lot of commented-out CSS. I'd recommend removing that, especially if you're planning on using this as a portfolio project. As a side note, I also saw a lot of padding-top and padding-bottom that were the same value. If you want to help consolidate those, you can use the padding-block property to combine them (padding-inline does the same for left and right, and the same rules apply to margins)!
All in all, great job!
@riwepo
Submitted
One thing I couldn't figure out, when you display an SVG by importing it as a ReactComponent, (used when you need to style it, like changing the fill color etc.) how do you then center the SVG inside the component? Mine kept appearing at the top left.
@ryanbradley-webdev
Posted
Looks pretty good! On your question, I have two things.
First, if you have a single-use svg you can absolutely copy-paste the html directly where you need it, and then apply a className prop to style it directly with css.
Second, if you're using a component to just return the SVG (I do this most of the time) it shouldn't change the styling directly. If the return statement looks like this:
return (
<svg> ... </svg>
)
It will be the same as putting it directly in the html. If you wrap it in another component it may alter your styling if you have any styles already applied to those components.
If you don't already use this tool, Chrome and FireFox have developer tools built in that can help you identify styling issues like this and make life a little easier.
I hope this helps with your future projects!
@aeorherhe
Submitted
@ryanbradley-webdev
Posted
This looks great! The only note I have is from viewing this on my phone in landscape. It might be of use to add a little top and bottom margin to the main component for smaller screen sizes to make sure it doesn't feel a little compacted, but otherwise it looks spot on!
I did also want to throw in my two cents on React. As a tool it's never really "necessary" to use it, it's all up to what you're comfortable using and what allows you to make the product as well as you can. If you want to practice it, I highly encourage you to use it for everything you can, even if it's not terribly complex or you think it's overkill. You'll definitely gain experience, and if it turns out it's not for you then that's great!
Marked as helpful
@StevetheRebel
Submitted
Read somewhere that it's best practice to be designing first for mobile then making sure that it's responsive to other device screens. Is this true and how would one go about doing it? This is because I still have a bit of a problem with the @media and screen and the like.
@ryanbradley-webdev
Posted
I believe that mobile-first development is definitely a smart choice. To add on to what Jordan said, I've found that developing for small screens first will also help you avoid headaches down the road. Think of it as adding complexity vs trying to tame complexity. Mobile designs are usually much simpler and more linear in their flow, whereas desktop layouts are usually laid out in grids and columns and have much more intricate designs. It's much easier to start simple and add on to a mobile design as you expand your layout than it is to try and remove elements from a complex desktop layout. At least most of the time that's been my experience.
I hope this helps!
Marked as helpful
@manavss
Submitted
Everything seems fine in the development on large screen but for some reason the layout break after uploading to netlify.Do you know the reason?Any feedbacks would be appreciated.
@ryanbradley-webdev
Posted
Hi there, this looks great! As for your question on the layout, I'm not very knowledgeable on Tailwind, but it looks like there's a conflict with the flex container housing your overview icons. My suggestion would be to add "justify-content: space-between" on that container, and then playing around with different flex-basis values on its children to find a good layout you like.
I hope this helps!
Marked as helpful
@Csabooo
Submitted
Hi All,
Huh... It was not too easy for me.
The JS and CSS code was a little hard and tricky.
Best regards,
Csaba
@ryanbradley-webdev
Posted
Hey there, great work on this project!
I was just looking through your code and noticed the layout regrading the 'header', 'main', and 'footer' sections in the markup. If this were a full-page project, I probably wouldn't have this advice, but since it's an 'article preview' project I feel that it's necessary.
The use of a 'footer' tag for the bottom portion of the card's content seems a little out of place. The footer should be the bottom-most part of the page, in your case the statement referencing the challenge and you as the author. This is a little arbitrary based on the content of the project, but in the future I'd recommend putting that content in a div or section as opposed to the footer tag.
Like I said, it looks great! Keep up the great work!
Marked as helpful
