All feedback is welcome
Hritick Bhushan
@hrk-berserker27All comments
- @iamcerebrocerberusSubmitted about 2 years ago@hrk-berserker27Posted about 2 years ago
Hi there, you did a great job in attempting this challenge. Following are my suggestions to improve your work:
- You can add functionality to your calculator using some JS;
- Adding appropriate hover effects on the buttons. Check out this for your inspiration: https://codepen.io/mjijackson/pen/xOzyGX. Check out this calculator which I built: https://codepen.io/madforstudyguy/pen/vYpWbVm
Marked as helpful1 - @hrk-berserker27Submitted about 2 years ago@hrk-berserker27Posted about 2 years ago
What is wrong with the screenshot? Anyone who has any idea about this glitch please share your thoughts.
0 - @yacineKahlerrasSubmitted over 2 years ago
any feedback is welcome 🤗
@hrk-berserker27Posted over 2 years agoHi there, you did a great job in completing this challenge. Here are some suggestions which will help you improve it by a notch:
- Try not to put the main element inside a section as it makes its use redundant. Try using this: <main> <header></header> <section></section> <section></section> <footer></footer> </main> It is one of many examples of getting rid of one landmark accessibility issue. Check this for more information: "https://www.semrush.com/blog/semantic- html5-guide/"
- Use heading tags such that you don't end up using different heading levels with more than one difference between their levels. Always try to decrease the heading level by one or change the size of the heading in CSS. *My tip: use only h1 and change it's font-size if you find it confusing to use other lower levels of headings.
Marked as helpful1 - @ByteOverloadSubmitted over 2 years ago
Hello mentors. This is my Day 5 of leaning html and CSS and I really enjoyed doing this challenge with some custom features inside. Hope the community will like it ! I guess i didn't put the right font family or it was just my browser . Please suggest any improvements
PS : I loved the hover effects and transitions :D
@hrk-berserker27Posted over 2 years agoHi there, you did a great job in completing this challenge. Following are some suggestions that you can incorporate into your solution to improve its accessibility and sort out the HTML issues:
- Enclose the content inside the main tag because a web page should have at least one landmark.
- Add the alt attribute to your image tags so that it will remove the accessibility warnings you are getting in your report. For further read, check this article: "https://accessibility.psu.edu/images/imageshtml/";
- Don't use paragraph tag inside button or span tag as it has semantic value. Replace it with span elements or any inline elements such <em> or <i> tags and alter their display to block elements in CSS if needed. Styling tip:
- The box shadow on the card is not in sync with the background. Try changing it or removing it. (It will improve the look remarkably)
- Try using a lighter color for button text for better contrast from its background and adding a little more padding to the buttons.
- Try increasing the card's height or little padding to the bottom of the card (with the box-sizing property set to border-box) because the cancel button seems to be overflowing. *Don't forget to mark it as helpful if these suggestions helped to improve your work.
Marked as helpful0 - @lurah11Submitted over 2 years ago
any feedback is welcome :)
@hrk-berserker27Posted over 2 years agoHi there, some of the suggestions which can help you to improve your solution:
- Use the main tag to enclose your content because there should be at least one landmark on your website.
- Increase the padding to improve the look of your website.
- Remove the action attribute from the form tag as it is unnecessary and it is not good practice to have an empty action attribute on form tags. Check this article for more information: "https://stackoverflow.com/questions/1131781/is-it-a-good-practice-to-use-an-empty-url-for-a-html-forms-action-attribute-a". Additional tip: Try centering the outer div using various div centering techniques.
Marked as helpful0 - @iammiracle01Submitted over 2 years ago
Hi there, take a look at my order summary card solution and leave a feedback or review.Thanks
@hrk-berserker27Posted over 2 years agoHi there, you did a great job. Following are some suggestions that you can incorporate into your solution:
- Don't use the paragraph tag inside the span tag as it has the semantic meaning associated with it and refers to a paragraph. For further reading: "https://stackoverflow.com/questions/1908234/when-to-use-span-instead-p#:~:text=We%20actually%20use%20span%20element,has%20semantic%20meaning%20in%20HTML" try checking this on stack overflow.
- Try adding the attribution in the footer tag and putting it inside the main tag. It will remove the main landmark accessibility issue. For further read, check this: "https://www.w3.org/WAI/ARIA/apg/example-index/landmarks/main.html#tabpanel1". You can also add a different label and add the attribution in a different main tag with the "aria-labeled by" attribute with unique labels for each main tag as suggested in the article link above.
Marked as helpful0 - @PRAISE-C24Submitted over 2 years ago
Please review the responsive aspect of the page.
What do you think about the alignment of the boxes?
@hrk-berserker27Posted over 2 years agoHi Praise, it is quite responsive and looks good but you can improve it by a notch by adding following suggestions:
- Try separating the top-most heading into two sections(slightly light one and dark one) so that the screenshot matches.
- You can also reduce the gap between the cards (optional -> it really looks good with your own gap:) ).
- You can also increase the font-size for the headings(slight increase) and the following content("1 rem" would be fine) as it looks really small.
- Regarding accessibility issues: You can add semantic elements like <main> for landmark and <section> in place for <div> elements, try using headings with less difference in their comparative values like if you have used h1 then don't use h3 for separating typography, try using h2 or use h1 but change its font size. These are some of the suggestions which I thought might help you. Don't forget to mark it as helpful if you liked the feedback otherwise its okay:).
Marked as helpful0 - @CrypticMangoSubmitted over 2 years ago
I was not sure how I could achieve the "cut-off corner" look in the desktop view for the woman online image. If anyone knows how to get this to work let me know. I tried using z-index but I am not experienced enough to get it to work. Unless there is another way. Any suggestions are appreciated. Also anything wider than 1440px, the orange box will move because I used absolute positioning, How do I make it stay where I want it with all larger screen sizes? Thanks in advance. Other than that completing this challenge taught me about the <details> HTML tag and how to use it.
@hrk-berserker27Posted over 2 years agoFollowing are hacks that I used to get rid of these problems while completing the challenge:
- If I am not wrong, the "cutoff of the corner" you are talking about is the overflowing part of the woman illustration image. You can get it right by enclosing the illustration in a div and adding "overflow: hidden" in the styles. It will hide the overflow part.
- To not allow the absolute positioning to alter the position of the image on increasing the screen size, you can add the "position: relative" to the container relative to which you want to fix its position and set the top and left properties in "em" units which should work.
- You might have missed the background of the illustration SVG's section.
If it doesn't work, then you can check out my code.
Marked as helpful1