@zofiadobSubmitted almost 3 years ago
MikevPeeren
@MikevPeerenAll comments
- @MikevPeerenPosted almost 3 years ago
Hey π
Try not to add alt text to meaningless icons like the Facebook icon, a screenreader has no use for that.
Also try to diversify your html elements, for example using headers and using lists for the different data.
Other than that good job π
Marked as helpful0 - @AliRadmaneshSubmitted almost 3 years ago@MikevPeerenPosted almost 3 years ago
Hey π
Good job π
Try to keep in mind when using alts text if it adds any value to a screenreader for example the background images do not. In that case you can use alt="" as that would be better.
Also as a improvement you could've used a ul list instead of divs for the footer section.
0 - @b-a-merrittSubmitted almost 3 years ago
I would appreciate some feedback on how readable / usable my code is
@MikevPeerenPosted almost 3 years agoHey π
Looks good to me, the only thing I can think about is that you could perhaps have predefined arrays with key value pairs this way you can just select your element for example for the housselection like:
Sethouse = Array[num]
And perhaps it would be cleaner to have two states one for the name and image.
Also useStates should always be ordered at the top so the function in between is a no go.
I would also have maybe a custom hook for your localstoragd to handle that.
But for the rest good job
Marked as helpful1 - @xxxcrisxxxSubmitted almost 3 years ago
The first project I upload. I think I can do better any comments welcome thanks.
@MikevPeerenPosted almost 3 years agoHey π
Good job finishing this one π
My only comment would be to not skip headings as you are now going from h1 to h3 you should try to use h2 in that case.
0 - @xs30snwSubmitted almost 3 years ago
I didn't know how to make a semantically proper switch between 'daily', 'weekly' and 'monthly' data.
On one hand, it should be a navigation block, in which links are organized inside a list. But the challenge also has the feel of an app, where these buttons should rather be radio buttons.
How would you implement this?
@MikevPeerenPosted almost 3 years agoHey π
Try to make use of headers instead of using paragraphs for everything. Also try to add alt text to images that require it, if they don't require it use alt="".
For your question I personally used buttons for the interactions but a list would also be fine imo.
Marked as helpful1 - @TelrodanSubmitted almost 3 years ago@MikevPeerenPosted almost 3 years ago
Hey π
My only comment would be to add more descriptive alt txt for accessibility a d also to remove it for non saying images like the eye icon, for those your can use alt=""
0 - @moses0xSubmitted almost 3 years ago
Feedbacks are highly appreciated. Thanks :)
@MikevPeerenPosted almost 3 years agoHey π
For the overlay the color is now blueish you can use something like the following:
βbackground-colorβ: βhslβ(β277β, β64β%β, β61β%β);β β background-blend-modeβ: βmultiplyβ;
Marked as helpful0 - @yaakov04Submitted almost 3 years ago@MikevPeerenPosted almost 3 years ago
Hey π
Try to keep in mind when to add alt text to images and if it's of use for a sxreenreader. For example for these images it isn't. Also always start with h1 and go from there.
Marked as helpful1 - @ahmeDeid67Submitted almost 3 years ago
feedback please.
@MikevPeerenPosted almost 3 years agoHey π
Good job finishing this one π
The on hover for the nft image is wider than the image as the div surrounding it triggers it resulting in that div also having the bg color.
Furthermore I am missing alt text for the nft as that is important data to have for a screenreader.
Marked as helpful0 - @PrashantIndurkarSubmitted almost 3 years ago@MikevPeerenPosted almost 3 years ago
Hey π
I am missing a bit of margin at the bottom so that it isn't clamped to the bottom of the div. Also the view code is linked to your profile so it's hard to find the repo.
0 - @developertarikSubmitted almost 3 years ago
I know that I have many shortcomings but I really tried hard to make the site look like I am waiting for your help I need help I am new to the front end field I am waiting for your opinions thank you in advance
@MikevPeerenPosted almost 3 years agoHey π
Good job finishing this one π
Try to add the font as is the same as the design it usually is written in the style guide. You can then import it as its described on Google fonts. https://fonts.google.com/
I am also missing the overlay over the image you can do it as follows:
βΒ Β βbackground-colorβ:Β βhslβ(β277β,Β β64β%β,Β β61β%β); βΒ Β βbackground-blend-modeβ:Β βmultiplyβ;
This needs to happen in css tho where you load the image in css.
Marked as helpful1 - @Abdullahi-1998Submitted almost 3 years ago@MikevPeerenPosted almost 3 years ago
Hey π
Good job looks good on mobile.
However try to start with header 1 and increment it rather than starting with a header 2. Also try to double check your code as you copied your alt text to multiple images.
Marked as helpful1
