ADEYEMO AYOMIDE O.• 340
@femmytedrey
Submitted
Charles Mambo
@charlesmambo
All comments
- Charles Mambo• 650
@charlesmambo
Posted
Hi, I hope you doing well, I was going through your project and I have a few suggestions.
<p style="letter-spacing: 5px; font-size: 10px; font-weight: 600; color: rgb(138, 138, 138)">PERFUME</p> I don`t think necessary to inline CSS since you have got CSS external folder. just try to use external CSS so that you can keep your code clean or if you want to change a certain area of your code and they do not change using classes you switch to ids.And here
<div class="price"> <p class="currentprice">$149.99</p> <p class="previousprice">$169.99</p> </div> your code is okay but you can simply achieve this by using a <span> like so <p class="currentprice">$149.99 <span>$169.99</span></p> if you want to add some changes to that text just give your span tag a class and edit on your CSS.And here .right{ height: 100%; width: 250px; background: white; border-radius: 8px; padding: 25px;
} Be really careful when giving your height and width values, do not pixels and percentage. if you are going to use pixels just sticky with the pixels, but I encourage you to use percentages, not pixels and let`s say you give your height:20%; make sure your width is also width: 20%;
Other than that everything is good #keep going
let me know if this helped
Marked as helpful
0 - payalpagaria• 70
@payalpagaria
Submitted
Hey Folks , I would love to have reviews on this solution, Any feedback ? please also do provide your suggestions ..!
Charles Mambo• 650@charlesmambo
Posted
Hey what's up buddy your looks great but I think it will be look great perfect if you add padding bottom maybe 8px to your cc container or you can increase the height because the last button is also at edge of your container.
cc{ height:520px; width: 350px; background-color: hsl(225, 100%, 98%); border-radius: 10px ; }
0 - arielaortin• 20
@arielaortin
Submitted
Charles Mambo• 650@charlesmambo
Posted
Hey brother congratulations Your project looks great, but I think you need to add little bit of padding top and bottom maybe 5px top and 5px bottom .button { display: flex; flex-direction: column; justify-content: center; margin-top: 25px; margin-left: 40px; margin-right: 40px; position: relative;
}
0 @GsaxVibe
Submitted
challenging but really interesting. would appreciate some feedbacks and tips on how it could have been better
Charles Mambo• 650@charlesmambo
Posted
Hey bro congratulations for completing this challenge I've few things I think you should add Add margin between the music icon and the annual text and also add a padding-bottom to text container .container { background-color: white; margin:20px; border-radius: 5px; box-shadow: 3px 3px 3px hsl(228, 45%, 44%) padding: 40px; }
Marked as helpful
1
