Elo• 120
@Elomolina
Submitted
Toluwaa
@Toluwaa-o
All comments
- Toluwaa• 1,040
@Toluwaa-o
Posted
Congratulations on completing the challenge.
Considering using <p> tags instead. Only use h1, h2, h3 etc if it's a heading. I would recommend you use h1 for the name and h2 for the 'work', 'exercise' etc.
Make sure you check the accessibility report provided by frontend mentor so you know if you need to make any changes
0 - Amira Lee• 10
@amiralee97
Submitted
I couldn't get my card to justify center, so I added a margin-top to remedy this. What is the cause of that happening? And is there a better solution? I wasn't sure to remove the space on the sides of the image without it affecting the border radius, is there a solution for this?
Toluwaa• 1,040@Toluwaa-o
Posted
Concerning your question, this should work:
body { max-width: 100vw; min-height: 100vh; display: grid; place-content: center; }
Max-width specifies the maximum width of the page and 100vw is a relative unit so it adapts to each device. Likewise min-height. It specifies the minimum width of the page. Setting it to 100vh means it will always take up the whole screen. VW means view width and VH means view height. For more info https://www.w3schools.com/cssref/css_units.php
You don't have to use the display grid/place content i used. But i suggested it because it takes less time to write and it has the same effect as using both justify-content & align items.
And lastly, I'd recommend adding padding to the container instead of adding it to each individual element in the container, that way they all align properly.
I hope these suggestions are helpful, let me know if you have any other question.
Marked as helpful
1 - 0la-lekan• 30
@0la-lekan
Submitted
I had issues with setting the transition on the "Add to Cart". I will appreciate If I can get help with it.
Toluwaa• 1,040@Toluwaa-o
Posted
Congratulations on completing the challenge. Concerning your question, use something like this:
button { transition: background-color 3s ease; }
button:hover { background-color: /color you want to transition to/; }
The transition property takes 3 parameters. First, the property you want to transition, the amount of time you want the transition to take and lastly, the transition style. You can read more here https://www.w3schools.com/css/css3_transitions.asp
The :hover pseudo selector is used to style an element when you hover over it. For more info, read about it here https://www.w3schools.com/cssref/sel_hover.php
0 @Rickylart
Submitted
Toluwaa• 1,040@Toluwaa-o
Posted
Congratulations on completing the project. A few suggestions, use font-weight in css instead of the <b> tag to make texts bolder.
Make use of a level one <h1> heading instead of <p> for the heading. You must have one level one heading on your webpage for better accessibility.
And lastly, wrap the project in a <main> tag and the bottom in a <footer>. I would also recommend using max-width on the card so it maintains a good shape even on large devices
Marked as helpful
0- Ketlly Azevedo• 10
@ketwy
Submitted
Toluwaa• 1,040@Toluwaa-o
Posted
Parabéns por concluir o desafio. Uma pequena sugestão, use o h1 em vez do h3. Você sempre deve ter um elemento h1 em seu projeto. Para mais informações, confira aqui: https://dequeuniversity.com/rules/axe/4.3/page-has-heading-one?application=axeAPI
E sempre verifique o relatório de acessibilidade fornecido pelo mentor do frontend
0 - Łukasz• 20
@Lucas1716
Submitted
Please give me a feedback!
Toluwaa• 1,040@Toluwaa-o
Posted
Hello, congratulations on completing the challenge.
Consider using the <del> tag on the old price. That tag automatically strikes through whatever text is between it. For more info, you can read about it here : https://www.w3schools.com/tags/tag_del.asp
Also, consider increasing the line-height of the paragraph so your design looks closer to the original design.
And lastly, consider adding an alt text to the cart image. For more info on the alt attribute, you can read here: https://www.w3schools.com/tags/att_img_alt.asp
Well done once again.
Marked as helpful
1 - Tiasha Halder• 20
@htiasha
Submitted
Could not add the author section and the <hr> didn't work :(
Toluwaa• 1,040@Toluwaa-o
Posted
Hello, instead of using a <hr>, you can wrap the 'eth' div and 'days' div in one div, and use a border-bottom on that. It has the same effect. Something like this:
<div class="stats"> <div class="eth"> <img src="images/icon-ethereum.svg" alt=""> <p>0.041 ETH</p> </div> <div class="days"> <img src="images/icon-clock.svg" alt=""> <p>3 days left</p> </div> </div>/* css */ .stats { border-bottom: 1px solid gray; }
And concerning adding the author, i would suggest creating a div tag for the bottom and put the author img and the other details in it. Something like this:
<div class="author-details"> <img src="" alt="Author"> <p>Creation of <span>Jules Wyvern</span></p> </div>I hope this is helpful. Let me know if you have any other questions.
Marked as helpful
0 - Muhammad Owais• 250
@ProgrammerOwais
Submitted
HI I just completed this challenge, plz check it out & let me know what else I need to improve, & Also I added the "key pressing" functionality too, to make it more unique. Plz check it & let me know.
Toluwaa• 1,040@Toluwaa-o
Posted
Hello. Congratulations on completing the challenge. I would suggest not saving the theme in local storage because on loading the page, the first two themes are the same.
Concerning the switch, i used a flex to create mine. Then i used a javascript condition to check the if its justified to the flex-start, center or flex-end. Then i set the theme accordingly.
Also, the style guide provides a font-style, that will make your work look closer to the design. Let me know if you have any questions or if you want me to elaborate on any of these suggestions
Marked as helpful
0 - Mapopo• 30
@Taku20202
Submitted
Don't know why but cannot centre divs easily.
Found it difficult to target array (buttons in ("body .rating") and apply 'onclick' in javascript. Apparently you can't apply 'onclick' to querySelectorAll etc. So you need to create a for loop / function and apply it to that? Any feedback would mean a lot!
Toluwaa• 1,040@Toluwaa-o
Posted
Well done on completing the challenge.
Concerning centering divs, you can use a few different methods. The first one is to set the margins to auto, the second method requires setting the display property to 'grid' and then setting 'place-content' to 'center'. The third method requires setting the display property to 'flex' and then setting the 'justify-content' and 'align-items' property to center. https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout/Aligning_Items_in_a_Flex_Container
Concerning your other question, querySelectorAll returns a nodeList. So you can either use a for loop or you can use the index (first one is 0) to select individual items in it. You can find more information here: https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelectorAll
Marked as helpful
1 - Chinedu• 210
@ChineduCode
Submitted
How can I align the days properly with the chart?
Toluwaa• 1,040@Toluwaa-o
Posted
Congratulations on completing the challenge.
Concerning your question, try setting the width of the days to the same width as the bars. Then use text-align to align the days to the center.
0
