@0marD
Submitted
This was quite a complex challenge for me, but I did it, although I know it could have been done in a better way So any suggestion is welcome.
Looking to hire developers?
@PhoenixDev22
@0marD
Submitted
This was quite a complex challenge for me, but I did it, although I know it could have been done in a better way So any suggestion is welcome.
@PhoenixDev22
Posted
Hi Omar Díaz Hernández,
Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:
<h1>
for class="main-cover__contents-heading"
and <h2>
for class="footer-side__data__heading"
<nav>
. As it is, assistive technology user won’t announce the button related to the <nav>
. And this is confusing and not good for the user.Toggle Element:
<button>
with type=”button”.1- The button needs to have anaria-label
attribute or an sr-only
text that describes the button purpose. For example, you can have: aria-label='Mobile Navigation Trigger'
or 'Open Menu.’
2- Adding aria-expanded
to the button, that way the user will be able to know that the button content controls is expanded or collapsed. At first, it has the “false” as a value then you use JavaScript to change the value.
3- You should use aria-controls
attribute on the toggle element, it should reference the id
value of the <ul>
element.
class="card-contents"
, you should never use <div>
and <span>
alone to wrap a meaningful content. Just keep in mind that you should usually use semantic HTML in place of the div tag unless none of them (the semantic tags) really match the content to group together. By adding semantic tags to your document, you provide additional information about the document, which aids in communication.alt
to it with an aria-hidden=”true”
to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative images for example.<nav >
landmark to wrap the footer navigation. Then you should add aria-label=”secondary “
or aria-label=”footer”
to it. A brief description of the purpose of the navigation, omitting the term "navigation", as the screen reader will read both the role and the contents of the label.nav
element in the header could use an aria-label="primary"
or aria-label=”main”
attribute on it. The reason for this is that, you should add the aria-label
for a nav element if you are using the nav more than once on the page.You can read more in MDNaria-label
or sr-only
text indicate where the link will take the user. Then you set aria-hidden =”true”
and focusable=”false”
to the svgs to be ignored by assistive technology .Hopefully this feedback helps.
Marked as helpful
If I could get some feedback about best practices I would appreciate it a lot!
@PhoenixDev22
Posted
Hi Marco Lizardo Del Riego,
Congratulation on finishing your first challenge.
Great job on this one! you have already received some helpful feedback which is nice to see , just going to add some suggestions as well if you don't mind:
aria-hidden=”true”
and focusable=”false”
to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative svgs.rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.hopefully this feedback helps.
@thelino3
Submitted
the grid i was did not know i am not learn enough Css grid
@PhoenixDev22
Posted
Hi ahmed,
Well done! I have some suggestions regarding your solution if you don't mind:
rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.object-fit: cover;
to the image which sets how the image should be resized to fit its container. object-fit: cover;
maintains its aspect ratio while filling the element's entire content box.Overall, great work! hopefully this feedback helps.
@iamthanuj
Submitted
Making QR code component Using HTML and CSS only
@PhoenixDev22
Posted
Hi Thanuja Fernando,
Excellent work! I have some suggestions regarding your solution:
Consider using min-height: 100vh
instead of height: 100%
to the body , that let the body grows taller if the content of the page outgrows the visible page.
max-width
to the card in rem
.Links must have discernible text also Check the footer's link , there are two nested links.
After , you fix the issues, you can generate another report for your solution.
Hopefully this feedback helps.
@WelangaiEric
Submitted
how is my positioning
@PhoenixDev22
Posted
Hi Welangai Eric,
Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:
HTML
<h1>
. The <h1>
is most commonly used to mark up a web page title. This challenge is supposed to be one component of a web page. To tackle the accessibility issue in the report , you may use an <h1>
visually hidden with class=”sr-only”.
You can find it here.for this imagine what would happen when you click on the image, there are two possible ways:
1: If clicking the image would show a popup where the user can see the full NFT, here you use <button>
.
2:If clicking the image would navigate the user to another page to see the NFT, here you can use <a>
.
You should have used <a>
to wrap Equilibrium #3429
and Jules Wyvern
too.
Sr-only
text, an aria-label
or alt
text that says where that link takes you.alt=""
and add aria-hidden="true"
attributes to make all web assistive technologies such as screen reader ignore those images in icon-view, icon-clock, icon-ethereum
.Jules Wyvern
. Read more how to write an alt text .<p>
instead of <h4>
in <h4> 0.041 ETH</h4>
.::before, ::after
. You can use pseudo-elements to change the teal background color to hsla. Then the opacity can be changed from 0 to 1 on the pseudo element on the hover. Also using pseudo elements makes your HTML more cleaner as there's no need for extra clutter in the HTML.rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.CSS
min-height: 100vh
instead of height: 100vh
to the body , that let the body grows taller if the content of the page outgrows the visible page. width:350px;
an explicit width is not a good way to have responsive layout . Consider using max-width
to the card in rem
.height: 600px;
It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height.height: 98%
.Hopefully this feedback helps.
Marked as helpful
@VaporDusk
Submitted
How can I improve this project?
@PhoenixDev22
Posted
Hi VaporDusk,
Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:
HTML
<h1>
. The <h1>
is most commonly used to mark up a web page title. This challenge is supposed to be one component of a web page. To tackle the accessibility issue in the report , you may use an <h1>
visually hidden with class=”sr-only”.
You can find it here.aria-hidden="true"
attribute to make all web assistive technologies such as screen reader ignore those images .CSS
min-height: 100vh
for the <body>
add a little padding to the body that way it stops the card from hitting the edges of the browser.marin-bottom: ;
only the link wouldn't need it and use margin-top:auto
on the learn more
link that will push it to the bottom of the cards.line-height: 48px
Use a unitless line-height value to Avoid unexpected results. You can read more in mdnborder-radius
and overflow hidden
to the main container that wraps the three cards so you don't have to setborder-radius
to individual corners.max-width
to the component that wraps the three cards in rem
.Hopefully this feedback helps.
Marked as helpful
@Rikvdev
Submitted
I tried to make this with dinamic grid using auto-fit , without media queries, but I had some errors with padding and container width, so I did it with media queries.
Did I do it right?
Are there any errors?
Where could I improve to make it better?
I would like to read your opinion :)!
@PhoenixDev22
Posted
Hi Riikis,
Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:
HTML
<h1>
. The <h1>
is most commonly used to mark up a web page title. This challenge is supposed to be one component of a web page. To tackle the accessibility issue in the report , you may use an <h1>
visually hidden with class=”sr-only”.
You can find it here.aria-hidden="true"
attribute to make all web assistive technologies such as screen reader ignore those images .rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.line-height: 1.8rem;
Use a unitless line-height value to Avoid unexpected results. You can read more in mdnYou absolutely did great job readable and reusable code.
Hopefully this feedback helps.
@Webdevsonu
Submitted
Here another preview cards I found it easy to make for the desktop screen size, but I found a little difficulty to make it switch the border-radius for small screen size as it shrinks. And I used "flex" to arrange the cards but I found out it wasn't perfect at some point while shrinking. So will you suggest some easy methods to fix it and how to auto adjust those border-radius for small screen.
@PhoenixDev22
Posted
Hello Webdevsonu,
Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:
HTML
<div>
and <span>
alone to wrap a meaningful content. Just keep in mind that you should usually use semantic HTML in place of the div tag unless none of them (the semantic tags) really match the content to group together. By adding semantic tags to your document, you provide additional information about the document, which aids in communication. <h1>
. In this challenge, as it’s not recommended to have more than <h1>
, you may use<h1>
visually hidden with class=”sr-only”. You can find it here then you can use <h2>
instead of a generic div.aria-hidden="true"
attribute to make all web assistive technologies such as screen reader ignore those images .<br>
, using <br>
is not only bad practice, it is problematic for people who navigate with the aid of screen reading technology. Screen readers may announce the presence of the element. This can be a confusing and frustrating experience for the person using the screen reader. You can read more in MDN.<br>
to increase the gap between lines of text
Or make empty space between elements, use padding / margin
styling via CSS. And about using <br> in the <p>
to make the paragraph break in new line, You may use max-width
to <p>
and remove those <br>
.rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.border-radius
and overflow hidden
to the main container that wraps the three cards so you don't have to setborder-radius
to individual corners.line-height:22px
Use a unitless line-height value to avoid unexpected results. You can read more in mdnheight: 350px;
It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height.Hopefully this feedback helps.
Marked as helpful
If anyone has any feedback on things I could do better, please let me know!
@PhoenixDev22
Posted
Hi Michael,
Congratulation on finishing this challenge.
Great job on this one! I have few suggestions regarding your solution:
<h1>
it is recommended not to have more than one h1 on the page. Multiple <h1>
tags make using screen readers more difficult, decreasing your site’s accessibility. In this challenge, as it’s not a whole page, you can have<h1>
visually hidden with sr-only
. Then you can swap those <h1>
with <h2>
.aria-hidden="true"
attribute to make all web assistive technologies such as screen reader ignore those images . <a
. For future use , it's a good habit of specifying the type of the button to avoid any unpredictable bugs.1. Actions where users affect the website’s back-end or front-end.
2. Actions where users won’t affect the website at all.
Action where users affect the website itself is where you use a button. For example, sign-up and purchase actions are often buttons. The user in these situations are creating a new account and completing a monetary transaction, which are actions that affect the website’s back-end. Creating new posts or making comments are actions that change a website’s content and what the user sees.
Actions where users won’t affect the website are where you use a link. These actions that take users from one page to another without changing anything on the website’s back or front-end.
border: 2px solid transparent;
to the regular state. This way when the hover on the buttons , it doesn't add an additional 4 pixels to the height and width making the elements shift.border-radius
and overflow hidden
to the main container that wraps the three cards so you don't have to setborder-radius
to individual corners.Hopefully this feedback helps.
Marked as helpful
@lunk-kml
Submitted
I think I make it look as close as possible. I still think my code is a little messy.
Feel free to review my code and preview the site. I welcome any feedbacks, tips, or ideas. I'm still learning.
!! Please let me know if my code is a mess or not lol
@PhoenixDev22
Posted
Hi KL,
Congratulation on completing another frontend mentor challenge.
Great job! i have some suggestions regarding your solution:
<main>
landmark for the main body content and<footer>
for the attribution as HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.<h1>
it is recommended not to have more than one h1 on the page. Multiple <h1>
tags make using screen readers more difficult, decreasing your site’s accessibility. In this challenge, as it’s not a whole page, you can have<h1>
visually hidden with sr-only
. Then you can swap those <h1>
with <h2>
.aria-hidden="true"
attribute to make all web assistive technologies such as screen reader ignore those images .button
must not appear as a descendant of the a
element. In this challenge, what would happen when the user click those learn more? In my opinion, clicking those "learn more" would likely trigger navigation not do an action so button elements would not be right. So you should use the <a
. For future use , it's a good habit of specifying the type of the button to avoid any unpredictable bugs.1. Actions where users affect the website’s back-end or front-end.
2. Actions where users won’t affect the website at all.
Action where users affect the website itself is where you use a button. For example, sign-up and purchase actions are often buttons. The user in these situations are creating a new account and completing a monetary transaction, which are actions that affect the website’s back-end. Creating new posts or making comments are actions that change a website’s content and what the user sees.
Actions where users won’t affect the website are where you use a link. These actions that take users from one page to another without changing anything on the website’s back or front-end.
rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.min-height: 100vh
for the <body>
add a little padding to the body that way it stops the card from hitting the edges of the browser. width: 62.5rem;
an explicit width is not a good way to have responsive layout . Consider using max-width
to the card.height: 32.1875rem;
- It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height.line-height: 25px;
Use a unitless line-height value to Avoid unexpected results. You can read more in mdnHopefully this feedback helps.
Marked as helpful
@cassiality
Submitted
All suggestions are welcome!
@PhoenixDev22
Posted
Hi Cassia Moraes,
Congratulation on completing anther frontend mentor challenge. I have some suggestions regarding your solution:
<footer>
landmark for the attribution , it should live outside the the <main> as landmarks allow screen reader users to navigate through sections of your website by skipping to content that interests them. Landmarks could be seen as the logical layout of the website's UI, which is divided into e.g. header, navigation, main content, and footer. So the usage makes sense in any case. <h1>
. The <h1>
is most commonly used to mark up a web page title. This challenge is supposed to be one component in a web page. To tackle the accessibility issue in the report , you may use an <h1>
visually hidden with class=”sr-only”.
You can find it here.aria-hidden="true"
attribute to make all web assistive technologies such as screen reader ignore those images . <a
. For future use , it's a good habit of specifying the type of the button to avoid any unpredictable bugs.rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.border-radius
and overflow hidden
to the main container that wraps the three cards so you don't have to setborder-radius
to individual corners.Aside these, your solution looks great. Hopefully this feedback helps.
Marked as helpful
@Nondaba
Submitted
Aligning the attribution class at the bottom was a challenge. How can I place attribution at the bottom while using flex box?
@PhoenixDev22
Posted
Hello Real uNondaba,
Well done! I have some suggestions regarding your solution if you don't mind:
<h1>
it is recommended not to have more than one h1 on the page. Multiple <h1>
tags make using screen readers more difficult, decreasing your site’s accessibility. In this challenge, as it’s not a whole page, you can have<h1>
visually hidden with sr-only
. Then you can swap those <h1>
with <h2>
.aria-hidden="true"
attribute to make all web assistive technologies such as screen reader ignore those images .<a>
. For future use , it's a good habit of specifying the type of the button to avoid any unpredictable bugs.border: 2px solid transparent;
to the regular state. This way when the hover on the buttons , it doesn't add an additional 4 pixels to the height and width making the elements shift.line-height: 1.25rem;
Use a unitless line-height value to avoid unexpected results. You can read more in mdnOverall , your solution looks great. Hopefully this feedback helps.
@vyigit
Submitted
only the desktop part of my work.
@PhoenixDev22
Posted
Well done! I have some suggestions regarding your solution if you don't mind:
Add to cart
is much likely to be a <button>
with type="submit"
instead of <a>
, in a <form>
. To know when to use one or the other in a specific situation, you must understand that every action on site falls under two different categories:1. Actions where users affect the website’s back-end or front-end.
2. Actions where users won’t affect the website at all.
Action where users affect the website itself is where you use a button. For example, sign-up and purchase actions are often buttons. The user in these situations are creating a new account and completing a monetary transaction, which are actions that affect the website’s back-end. Creating new posts or making comments are actions that change a website’s content and what the user sees.
Actions where users won’t affect the website are where you use a link. These actions that take users from one page to another without changing anything on the website’s back or front-end.
<picture>
tag in HTML to specify image resources. The <picture>
tag contains<source>
and <img>
tags. This way the browser can choose the image that best fits the current view and/or device. If you have a small screen or device, it is not necessary to load a large image file. The browser will use the first<source>
element with matching attribute values, and ignore any of the following elements.The alternate text in the product image should not be empty it should describes the product.
<del>
tag is used to identify text that has been deleted from a document but retained to show the history of modifications made to the document. Strike through is a CSS property and does not carry any semantic meaning as inserted or deleted for screen readers.
For screen reader: <del>
deleted indicates text removed. In this instance, the two prices are read out which can be confusing.aria-hidden=”true”
to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative images. Remember the alternate text should not be hyphenated, it should be human readable.object-fit: cover;
to the image which sets how the image should be resized to fit its container. object-fit: cover;
maintains its aspect ratio while filling the element's entire content box.width: 800px;
an explicit width is not a good way to have responsive layout . Consider using max-width to the card in rem.height: 600px;
It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height.line-height: 25px;
Use a unitless line-height value to Avoid unexpected results. You can read more in mdnem
.Overall, great work! hopefully this feedback helps.
Marked as helpful
@felixmakinda
Submitted
I still could not figure out the relevance of the desktop and mobile designs. While I used the media query tag, I am not sure if I did the right thing and whether it was relevant. Kindly clarify.
@PhoenixDev22
Posted
Hi Felix Makinda,
Congratulation on completing your first frontend mentor challenge. I have some suggestions regarding your solution:
QR code to frontendmentor.io
not describes the image.min-height: 100vh
for the <body>
add a little padding to the body that way it stops the card from hitting the edges of the browser. No need for position absolute and the negative margins.Personally, I don’t restrict the width of the body element. If I need to restrict the width I use a container div with a max-width on it.
width: 300px;
an explicit width is not a good way to have responsive layout . Consider using max-width
to the card in rem
.height: 600px;
It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height.Overall, your solution looks great. Hopefully this feedback helps.
Marked as helpful
@ClassyKMR
Submitted
I tried to use BEM naming for CSS. Can someone confirm that it is correct?
@PhoenixDev22
Posted
hello ClassyKMR,
Congratulation on completing your first frontend mentor challenge. I have some suggestions regarding your solution if you don't mind:
<main>
landmark to wrap the main body content as HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.QR code to frontendmentor.io
not describes the image. <h1>
. In this challenge to tackle the accessibility issue, you may use<h1>
visually hidden with class=”sr-only”. You can find it here. Then you can use <h2>
instead of p class="block__text block__text--heading"
as you should have used header ( typography ) to emphasize primary information in the card.min-height: 100vh
for the <body>
add a little padding to the body that way it stops the card from hitting the edges of the browser. Remove those CSS from the html.width: 280px;
an explicit width is not a good way to have responsive layout . Consider using max-width
to the card in rem
.Aside these, great job. Hopefully this feedback helps.
Marked as helpful
@Mohammedsalih1
Submitted
nothing was diffcult it was easy and fun i enjoyed making this project, no i am 100% sure about it, no thank you.
@PhoenixDev22
Posted
Hi Mohammedsalih1,
Congratulation on completing your first frontend mentor challenge. I have some suggestions regarding your solution:
<main>
landmark to wrap the main body content as HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.QR code to frontendmentor.io
not describes the image. <h1>
. In this challenge to tackle the accessibility issue, as it’s supposed to be a part of a whole page, you may use<h1>
visually hidden with class=”sr-only”. You can find it here. Then you can use <h2>
instead of <h4>
. Remember to use the headers in a chronological order.min-height: 100vh
for the <body>
add a little padding to the body that way it stops the card from hitting the edges of the browser. No need for position absolute.Personally, I don’t restrict the width of the body element. If I need to restrict the width I use a container div with a max-width on it.
width: 250px;
an explicit width is not a good way to have responsive layout . Consider using max-width
to the card in rem
.Overall, your solution looks great. Hopefully this feedback helps.
Marked as helpful
@ch-andrew
Submitted
How can I make the product image even more responsive in between resizing of desktop to mobile view?
@PhoenixDev22
Posted
Hello ch-andrew,
Well done! I have some suggestions regarding your solution if you don't mind:
<picture>
tag in HTML to specify image resources. The <picture>
tag contains<source>
and <img>
tags. This way the browser can choose the image that best fits the current view and/or device. If you have a small screen or device, it is not necessary to load a large image file. The browser will use the first<source>
element with matching attribute values, and ignore any of the following elements.<del>
tag is used to identify text that has been deleted from a document but retained to show the history of modifications made to the document. Strike through is a CSS property and does not carry any semantic meaning as inserted or deleted for screen readers.
For screen reader: <del>
deleted indicates text removed. In this instance, the two prices are read out which can be confusing. aria-hidden=”true”
to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative images. Remember the alternate text should not be hyphenated, it should be human readable.object-fit: cover;
to the image which sets how the image should be resized to fit its container. object-fit: cover;
maintains its aspect ratio while filling the element's entire content box.Overall, great work! hopefully this feedback helps.
Marked as helpful
@mnalii
Submitted
This is my first challenge submission. Feel free to give any feedback and advice. I am still confused converting the line-height value in figma to CSS unit as i am not quite sure what unit value in figma( for example the value in line height only showed 32 in figma, is it in 32px or another unit value? ). Also, anyone knows converting the value of paragraph spacing from figma into css properties/value ?
@PhoenixDev22
Posted
Hi Muhammad Nur Ali,
Congratulation on completing your first frontend mentor challenge.
Great work on this challenge ! I have some suggestions regarding your solution if you don't mind:
Add to cart
is much likely to be a <button>
with type="submit"
instead of <a>
, in a <form>
. To know when to use one or the other in a specific situation, you must understand that every action on site falls under two different categories:1. Actions where users affect the website’s back-end or front-end.
2. Actions where users won’t affect the website at all.
Action where users affect the website itself is where you use a button. For example, sign-up and purchase actions are often buttons. The user in these situations are creating a new account and completing a monetary transaction, which are actions that affect the website’s back-end. Creating new posts or making comments are actions that change a website’s content and what the user sees.
Actions where users won’t affect the website are where you use a link. These actions that take users from one page to another without changing anything on the website’s back or front-end.
<picture>
tag in HTML to specify image resources. The <picture>
tag contains<source>
and <img>
tags. This way the browser can choose the image that best fits the current view and/or device. If you have a small screen or device, it is not necessary to load a large image file. The browser will use the first<source>
element with matching attribute values, and ignore any of the following elements.<del>
tag is used to identify text that has been deleted from a document but retained to show the history of modifications made to the document. Strike through is a CSS property and does not carry any semantic meaning as inserted or deleted for screen readers.
For screen reader: <del>
deleted indicates text removed. In this instance, the two prices are read out which can be confusing. an aria-hidden=”true”
and focusable=”false
to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative images.font-size: 62%
it state that you should never change the root font size because it harms accessibility.object-fit: cover;
to the image which sets how the image should be resized to fit its container. object-fit: cover;
maintains its aspect ratio while filling the element's entire content box.line-height: 2.3rem;
Use a unitless line-height value to Avoid unexpected results. You can read more in mdnOverall, great work! hopefully this feedback helps.
@dieghi-capri
Submitted
All feedback is welcome
@PhoenixDev22
Posted
Hi diego,
Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:
HTML
<button>
.
2:If clicking the image would navigate the user to another page to see the NFT, here you can use <a>
.Sr-only
text, an aria-label
or alt
text that says where that link takes you.You should have used <a>
to wrap Equilibrium #3429
and Jules Wyvern
too.
<h1>
. The <h1>
is most commonly used to mark up a web page title. This challenge is supposed to be one component of a web page. To tackle the accessibility issue in the report , you may use an <h1>
visually hidden with class=”sr-only”.
You can find it here.According to mdn, <article>
HTML element represents a self-contained composition in a document, page, application, or site, which is intended to be independently distributable or reusable.
In this challenge, you have misused the <article>
as the NFT card is one component and all the content of blocks are related to a single subject or destination. You should remove all the article tags and I recommend to use <section>
for the whole component .
For any decorative images, each img tag should have empty alt=""
and add aria-hidden="true"
attributes to make all web assistive technologies such as screen reader ignore those images in icon-view, icon-clock, icon-ethereum
.
Profile images like that avatar are valuable content. The alternate text should not be empty.You can use the creator's name Jules Wyvern
. Read more how to write an alt text .
For middle part of the card , in each <li>
there should be <img>
and <p>
.
If you wish to draw a horizontal line, you should do so using appropriate CSS. You may remove the <hr>
, you can use border-top:
to the avatar's part.
Adding rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.
There are so many ways to do the hover effect on the image, The one I would use is pseudo elements::before, ::after
. You can use pseudo-elements to change the teal background color to hsla. Then the opacity can be changed from 0 to 1 on the pseudo element on the hover. Also using pseudo elements makes your HTML more cleaner as there's no need for extra clutter in the HTML
After you fix the issues, you can generate another report for your solution.
Hopefully this feedback helps.
@zakariabelassri
Submitted
Feedback would be welcome.
@PhoenixDev22
Posted
Hi Zakaria,
Congratulation on finishing this challenge. I have few suggestions regarding your solution:
HTML
<button>
.
2:If clicking the image would navigate the user to another page to see the NFT, here you can use <a>
.Sr-only
text, an aria-label
or alt
text that says where that link takes you.You should have used <a>
to wrap Equilibrium #3429
.
aria-hidden="true"
and focusable=”false”
attributes to make all web assistive technologies such as screen reader ignore those svgs.Jules Wyvern
. Read more how to write an alt text .<div class="card__line"></div>
, you can use border-top:
to the avatar's part.min-height: 100vh
instead of height: 100vh
to the body , that let the body grows taller if the content of the page outgrows the visible page.::before, ::after
. You can use pseudo-elements to change the teal background color to hsla. Then the opacity can be changed from 0 to 1 on the pseudo element on the hover. Also using pseudo elements makes your HTML more cleaner as there's no need for extra clutter in the HTML.Aside these, your solution looks great. Hopefully this feedback helps.
Marked as helpful
@Noseriousv
Submitted
Best practices
@PhoenixDev22
Posted
Hi Noseriouszv,
Congratulation on competing your first frontend mentor challenge.
Well done! I have some suggestions regarding your solution if you don't mind:
<main>
landmark to wrap the card as landmarks allow screen reader users to navigate through sections of your website by skipping to content that interests them. Landmarks could be seen as the logical layout of the website's UI, which is divided into e.g. header, navigation, main content, and footer. So the usage makes sense in any case. <h1>
. The <h1>
is most commonly used to mark up a web page title. This challenge is supposed to be one component of a web page. To tackle the accessibility issue in the report , you may use an <h1>
visually hidden with class=”sr-only”.
You can find it here.QR code to frontend mentor
not describes the image.<p>
instead of <h4>
. For future use , consider using the headers in a chronological order. How you order headings dictates how a screen reader will navigate through them. As you go down a level, the number should increase by one, like a numbered list within an outline.min-height: 100vh
for the <body>
add a little padding to the body that way it stops the card from hitting the edges of the browser. Then you can remove.Container-1
. height: 35.625rem;
It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height width: 21.875rem;
an explicit width is not a good way to have responsive layout . Consider using max-width
to the card instead.Hopefully this feedback helps.
Marked as helpful
@dboca93
Submitted
Hey there !
If anyone could please provide some feedback, I would really appreciate it :) I believe I've taken account of all the previous feedback I received for this challenge, however I did find the ARIA labelling a little confusing -- would love to know if I did this properly !?
The slideout footer at the bottom is meant only as a reference point for people to contact me personally. Let me know if this is inappropriate on front-end mentor.
Regards, Dilhan Boca
@PhoenixDev22
Posted
Hi Dilhan Boca,
Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:
HTML
<h1>
. The <h1>
is most commonly used to mark up a web page title. This challenge is supposed to be one component of a web page. To tackle the accessibility issue in the report , you may use an <h1>
visually hidden with class=”sr-only”.
You can find it here. Then you can use <h2>
instead of <h3>
as it's recommended use the headers in a chronological order.<button>
.
2:If clicking the image would navigate the user to another page to see the NFT, here you can use <a>
.You should have used <a>
to wrap Equilibrium #3429
and Jules Wyvern
too.
Sr-only
text, an aria-label
or alt
text that says where that link takes you.alt=""
and add aria-hidden="true"
attributes to make all web assistive technologies such as screen reader ignore those images in icon-view, icon-clock, icon-ethereum
.The icon view does not really need to be in the HTML. You can use CSS for it.
Profile images like that avatar are valuable content. The alternate text should not be selfie.You can use the creator's name Jules Wyvern
. Read more how to write an alt text .
The social links wrapping the icons should have aria-label
(as you did )or sr-only
text indicate where the link will take the user. Then you set aria-hidden =”true”
to the icons to be removed from the accessibility tree and ignored by assistive technology .
Hopefully this feedback helps.
Marked as helpful
@SimonMartorano
Submitted
All feedback is welcome. Thank you!
@PhoenixDev22
Posted
Congratulation on completing anther frontend mentor challenge.
Well done! I have some suggestions regarding your solution if you don't mind:
Add to cart
is much likely to be a <button>
with type="submit"
instead of <a>
, in a <form>
. To know when to use one or the other in a specific situation, you must understand that every action on site falls under two different categories:1. Actions where users affect the website’s back-end or front-end.
2. Actions where users won’t affect the website at all.
Action where users affect the website itself is where you use a button. For example, sign-up and purchase actions are often buttons. The user in these situations are creating a new account and completing a monetary transaction, which are actions that affect the website’s back-end. Creating new posts or making comments are actions that change a website’s content and what the user sees.
Actions where users won’t affect the website are where you use a link. These actions that take users from one page to another without changing anything on the website’s back or front-end.
<picture>
tag in HTML to specify image resources. The <picture>
tag contains<source>
and <img>
tags. This way the browser can choose the image that best fits the current view and/or device. If you have a small screen or device, it is not necessary to load a large image file. The browser will use the first<source>
element with matching attribute values, and ignore any of the following elements.<del>
tag is used to identify text that has been deleted from a document but retained to show the history of modifications made to the document. Strike through is a CSS property and does not carry any semantic meaning as inserted or deleted for screen readers.
For screen reader: <del>
deleted indicates text removed. In this instance, the two prices are read out which can be confusing.rel="noopener"
or rel="noreferrer"
totarget="_blank"
links. When you link to a page on another site using target=”_blank”
attribute, you can expose your site to performance and security issues.min-height: 100vh
for the <body>
add a little padding to the body that way it stops the card from hitting the edges of the browser.Overall, great work! hopefully this feedback helps.
Marked as helpful
@JheanKhendrick
Submitted
Feel free to notify me about things that could make this better. Also, a challenge suggestion is much appreciated.
Thank you!
@PhoenixDev22
Posted
Hello Jhean Khendrick,
Congratulation on finishing this challenge. I have few suggestions regarding your solution:
CSS
min-height: 100vh
instead of height: 100vh
to the body , that let the body grows taller if the content of the page outgrows the visible page.line-height: 1.5rem;
Use a unitless line-height value to Avoid unexpected results. You can read more in mdntext-align: center;
Aside these, great work! Hopefully this feedback helps.
Marked as helpful