@Bidhu1024Submitted 11 months ago
Please find free to check and suggest any ideas if you have.
Vijay K Naik
@KruzadeR-VictoRAll comments
- @KruzadeR-VictoRPosted 11 months ago
Hi @Bidhu1024 👋,
i saw your solution and you've done a good job.
Here are some issues i noticed you might consider :
- it's a good practice to wrap all ur contents in a main tag to avoid further accessibility issues, you can learn more about web accessibility here
- consider adding some padding around the card content as in the design
- also add the hover effect in active state of the card ( the "box-shadow" transition )
addition tip :
- while developing, you should always compare your solution with the given designs before submission to achieve best accuracy
Good luck with ur future developments ✌
0 - @codederkSubmitted over 1 year ago@KruzadeR-VictoRPosted over 1 year ago
Hello 👋, You've really done a good job on this challenge.All the HTML tags are well implemented and CSS is also perfect.Besides you should consider making the cursor 'pointer', when hover on the button.
0 - @PriyalloharSubmitted over 1 year ago@KruzadeR-VictoRPosted over 1 year ago
Hi there 👋,
I think you've uploaded solution of another challenge here, you should have a look into your solutions after you upload them and check if there is any accessibility or HTML issues.
1 - @A-BagoSubmitted over 1 year ago
Hello there, lovely folks!
This is my first experience working with JavaScript and, in particular, event listeners. I would sincerely appreciate any guidance or suggestions you could provide based on your experiences.
In addition, I'm trying to adhere to the BEM naming convention for classes. If you have any insights or constructive feedback about this aspect of my work, it would be most welcome.
Thank you in advance for your time and your valuable input.
@KruzadeR-VictoRPosted over 1 year agoHi there 👋,
You have done a good job with styling the UI, i'm really impressed with the approach.Maybe you should add some padding to the button with arrow inside and you're good to go.
And for the Logic of the calcultaion , you should refine that because it proceed to calculate even if the the values are invalid, which should'be verified before the calculation.And add proper validations and display error messages, if any.
If you're willing to learn Javascript basics, you can go for W3Schools or https://javascript30.com/ . You can always test and imporove your development skills with frontendmentor projects.. Keep Coding.. ✌
0 - @m0nagamalSubmitted over 1 year ago@KruzadeR-VictoRPosted over 1 year ago
Hi there 👋,
you have done a pretty good job, For the background illustration you should add
background-repeat:'no-repeat' & background-position:'top'to the <body> tag and change bg-size to 'contain' so that it takes the space to fit itself and doesn't repeat in some cases. Also don't use margin in ur <main> and set height to 100vh instead and you're good to go 👍.The bg illustration will look more centered that way1 - @karthik5860Submitted about 2 years ago
This is version 2 in this I have tried to solve various problem encountered in version 1 but following are doubts in this version:
- How to make the website look same in various browsers like chrome and Firefox?
- There are some problems in mobile version how to not overlap one div with other when one has position absolute?
@KruzadeR-VictoRPosted about 2 years agoHi karthikeya,
To answer your questions i'd suggest you to use html tags properly in your code so that you don't have to get confused while styling them and it will be really helpful even if you decide to change your styling later on.Make sure you use correct layouts in your code and it'll get you the same result in all the browsers.
To resolve the issue of overlapping you should first understand the use of positions, like whenever you're using absolute to an element make sure it's parent element has position: relative , otherwise i'll take the document as its parent and style the element accordingly and that's what happening with your solution.You shouldn't use position absolute with any element if not needed.To understant positions i would suggest you this (https://css-tricks.com/almanac/properties/p/position)
And you can resolve the accessibility issues by using a main tag in you code instead of a normal div and for the logo you can put it in a header tag as well and use css instead of ui libaries which will save you from HTML validation issue or you can use sass compiler if you like
I hope i've helped you with your issues and if you need some reference regarding the solultion you can check my solutions or others as well ..... Ty. Keep Coding.......
Marked as helpful0 - @RafaelHDSVSubmitted about 2 years ago@KruzadeR-VictoRPosted about 2 years ago
Hi Rafael, you've done a good job with your solution and you should do the mobile screen part as well . So for the accessibility issues you can use a 'main' tag instead of div and also i see that the logo is not visible due to the background you've set , you can fix that by using z-index to your logo. And try to make the typography of the design as accurate as possible like the font-family,font-size and line-height
Marked as helpful0 - @jeevatekSubmitted about 2 years ago
Any feedback is welcome
@KruzadeR-VictoRPosted about 2 years agoHi Jeeva,
you may consider the following changes :
- the accesibility issue is due to the h2 tag you have used for the heading of the product,you should use h1 tag to avoid the issue. For more details check https://dequeuniversity.com/rules/axe/4.1/page-has-heading-one
- card container is not centered in larger screens due to the max-width property you've set to the container
- And the product details page should contain half of the card and so is the product image
- You need to also check the typography for bigger screens like the 'perfume' and the 'description' part
Otherwise you've done a pretty good job on mobile part, Happy coding .. Ty
Marked as helpful0 - @ThePartnerDeveloperSubmitted about 2 years ago
How can I better approach putting the colored borders on the cards? I made an individual class for each card's border to change the color, but I'm sure there's a better way to do that without making separate classes or using inline.
What else can I improve on?
@KruzadeR-VictoRPosted about 2 years agoHi Connor,
i was checking out your solution and i noticed that it creates a horizontal scroll on larger screens beacuse of the fixed width of the body,instead you can set width to 100% or use max-width or overflow property to fix that.And for the colored borders you can use psuedo elements instead of classes.Other than that you've done a great job .
Marked as helpful1 - @JagroopNattSubmitted over 2 years ago@KruzadeR-VictoRPosted over 2 years ago
Hi Jagroop , i liked your approach towards this challenge and you got almost close with the design, but there are some styling issues that you should look at like the overflow caused when you click on all the questions ,the image layout shift and maybe the image part too, for the solution you can set to preview one question at time and avoid setting a fixed height to your grid layout.
And when i click on one question all down arrows are responding to that,you might wanna fix that as well.For the reference you can check out my solution.
Other than that you've done a good job, Happy Coding :)
0 - @KruzadeR-VictoRSubmitted over 2 years ago
i've made some changes to the product carousel for mobile devices instead of the given design pattern.( BTW i'll update that shortly ). And you can post your suggestion below.. Ty
@KruzadeR-VictoRPosted over 2 years agoif anyone could give me an idea of getting rid of these HTML validation issues, that'd be a great help. Ty..
0 - @azimifardousSubmitted over 2 years ago
The only problem I had through making this was using and inserting those backgrounds. Can anyone tell how did they able to do that?
@KruzadeR-VictoRPosted over 2 years agoyou've done a good job and for the backgrounds you can use them before and after the sections and you'll get the same design, for reference you can check mine.
0
