
Assurance Chioma Ikogwe
@Aik-202All comments
- @fkrunic@Aik-202
Wow! Nice work, I loved the feature of scanning the card you added, but:
- There's an issue with the "Card number"" expiration year" checks, even with the correct input, it keeps showing the error message.
- After every 4 numbers for the card number input field you can add an extra line of space.
- You can also restrict card name to letters only, card number, expiration date, and CVC to numbers only.
- You can use the
maxlength
attribute in you input fields to restrict the length of the input field.. for example CVC input field will have a maxlength` of 3.
These are my suggestions, great work once again!
Marked as helpful - @mukwende2000@Aik-202
Hi Mukwende, you did okay. I have some suggestions though.
- Change the
div
withclass container
tomain
, as every html page must have one main tag. - Remove one
h1
heading from your page. I suggest you remove this one with<h1 class="center">Get early access today</h1>
as they ought to be just one main heading. you can useh3
and style it the way you want. - You are right, the responsiveness is not quite there. I use
media queries
for responsiveness. You used@media screen and (min-width: 768px)
. What this means is that the code below it is for screens with size 768px and above, that is for ipads and desktop. That's by the way side. Although, the responsiveness is not the problem, there's atext overflow
from :<h1 class="center">All your files in one secure location, accessible anywhere.</h1>
And<p>It only takes a minute to sign up and our free starter tier is extremely generous. If you have any questions, our support team would be happy to help you.
I suggest you adjust that. - Then set
margin:0
for the body, so that there's no visible spacing around the page.
Hope this helps, do well to update your solution when you make these changes.
Marked as helpful - Change the
- @baskarancs15@Aik-202
Hi Baskaran, Nice work!. I have some suggestions though :
- Change the
div
withclass container
tomain
as every html page must have a main tag. - For the Mobile mode(
@media(max-width:600px)
), there's an issue, there's too much space at the top. Then I noticed fir the desktop you set the margin top and bottom to 20em withmargin-top: 20em auto
. Reduce the margin-top for the mobile mode ... It's too much. - I noticed you were actually using the margins above to center the container?. If that's right, it's better to set
justify-content: center
andalign-items: center
in your css. This will center your container. - Also, try and adjust the
max-width
of your container for mobile mode, they seem too big.
Hope this helps, please update your solution when you make the changes.
Marked as helpful - Change the
- @DavidQA71@Aik-202
Hi David, Nice work, you did justice to this. I have some suggestions though:
- Turn the
div
withclass container
tomain
tag as every html page must have amain
tag as it tells screen readers where the page starts from. - Then in each of your
section
... Turn the firstp
tags withclass p1
to h2 -h6 headings. As sections must begin with h2-h6 headings. That should solve some of your accessibility problems. Nice work once again. Hope this helps, you can update your solution once you've made these changes.
Marked as helpful - Turn the
- @Adrianmbugua@Aik-202
Hi Adrian, nice work! To answer your question the way to resize font awesome icons is to use
font size
. Using fa-2x in your class simply gives the icon a font size of 2em. this link will tell you more about sizing of font awesome icons . Also, for your accessibility issues, change the div with class main to themain
tag, as every page must have onemain
tag. Try also to center the button, it looks a little bit off in the mobile view.Hope this helps.
Marked as helpful - @lichtle@Aik-202
Hi, Leticia... Nice work, you did amazing... I see that you used an entirely different color theme, it's beautiful. I have some suggestions though...
- I'll like to ask, why did you use div for every content??. You can use the different levels of headings, the p tag, pre tag, for your content and style them as you like. I feel like the purpose of
div
is for grouping contents. So I'll advise you change the div with class name to anh1
tag. That will solve ya accessibility issues. - I also checked ya code, I don't see anything wrong with the way you wrote it tho. One thing to note is that if you are using the
justify-content: center;
to center a div, you also need to includealign-items: center
for it to work.
Nice work once again. Hope this helps.
Marked as helpful - I'll like to ask, why did you use div for every content??. You can use the different levels of headings, the p tag, pre tag, for your content and style them as you like. I feel like the purpose of
- @H7ioo@Aik-202
Hi, Omar! Wow! Nice work! I love the idea that you can see the choices of the house toggle until one is finally picked. It's really nice!!!. But for the mobile view it's not quite there... Some contents are overlapping and the score box is too big... And try changing the
section
tag tomain
tag. This is really nice. Well done!!!Marked as helpful - @Khanza2@Aik-202
Hi, Khan... Nice work!!!!. I have some suggestions though :
-
Try changing the
section
tag with class maincontainer tomain
tag, as every html page must have a main tag, as it helps screen readers know where the main content of the page is located. -
Remove the the href attributes that have no value from the anchor tags, that should sole some of your html issues.
-
For the mobile mode, it's not exactly pixel perfect, you can also reduce the size of the images as they appear too big. You can also reduce the font-sizes of the headings, they also appear too big.
Hope this helps.
Marked as helpful -
- @Contrerasdev@Aik-202
Hi Walter, Nice work!!!!. I have some suggestions though
- Change the
div
tag with class container tomain
, as there must be amain
tag in your html... - You can change the
button
tag to adiv
and give it aria role "button", this will help fix the html issue. i.e<div role = "button"></div>
- Instead of using
h1
tag for the striked price you can use thesup
tag , it's used for superscripted text in html... and it can be embedded in the firsth1
tag with class precio i.e<h1 class="precio" > $149.99 <sup>149.98</sup> </h1>
.. by doing this you will reduce the spacing between the two digits..
Marked as helpful - Change the
- @MIHI-bot@Aik-202
Hi Mihir, this is wonderful! Nice work! I have some suggestions tho.
-
Make sure to always put an alt text for all your images, I think that should fix most of your html and acessibility issues
-
For the ones that say you should add img src, I think the problem as to do with the way you wrote the src for the images. It ought to have been
./images/img.jg
as the images is in another folder. -
Use the
main
tag for the div with class modal as that is where your main content starts from. -
Instead of using
strike
tag, use this in your css :text-decoration: line-through
-
I also noticed that once I change the number of shoes I'm ordering, it remains unchanged in my cart.
-
Instead of using the
button
tag for your plus and minus icons, try using thediv
tag, and assigning the aria role button... i.e<div role = "button"> </div>
Hope this helps.
-
- @melco10@Aik-202
Hi Co, Nice work!!!!. But I have some suggestions. For the JavaScript,
- From the cards, it's already obviously that the user is only suppose to choose from 1 to 5, so there's no need testing for that. Rather it should be something like this.
if (!rateNum) //that is rateNum is undefined { //error message } else { //submit rating }
This way you get to provide a check for when the user does not enter any input.
For the html
-
Try changing that h2 heading to h1, it will solve ya accessibility issue as there must be an h1 heading in your html...
-
Add alt to all your images, you can see why here
-
I feel like the link google.com wrapped around the star image is not necessary. What were you trying to achieve with that?🤔🤔🤔
-
You can remove the link embedding the submit button, as it's not serving any purpose there.
Hope, this helps.
Marked as helpful - From the cards, it's already obviously that the user is only suppose to choose from 1 to 5, so there's no need testing for that. Rather it should be something like this.
- @AtulKumar0001@Aik-202
Hi Atul, this is pixel perfect!!!! 😍😍😍. But I have some suggestions that can help you with your accessibility issues.
-
You can change that div with class big container to main, as your code must have one main tag or you can add this to big container
role = 'main'
. Then the big container will be treated as main -
Change the h2 tag to h1, you can give it any font size in your css.
-
Attach
aria roles
to each of your divs. I think the aria role to be used for the div with class container is group i.erole = 'group'
. Click on this link to know more about aria roles
Hope this helps.
-
- @alanacapcreates@Aik-202
Hi Alana, Nice work!!!. To answer your question, I just completed this challenge and I used w3 schools method for the image overlay you can find that here. Hope it helps!
- @madegilangaditya@Aik-202
Hi, Gilang Nice one!!!. The responsiveness is on point, and your design is really nice, putting a bit of box shadow on the cards was a really great idea!!. But I have some suggestions
-
If the cursor is still on any input field and the confirm button is clicked , it doesn't work. Don't know y, maybe check your JavaScript
-
Try setting the height of the image to 100vh so that it takes the total height of the viewpoint.
-
You can use
max-length
attributes in your html to ensure that the user only enters 3 CVC numbers, 19 card numbers, 2 numbers for the month and year as well... -
I also noticed that there's no validation taking place for the card name field, the users are allowed to enter just anything. That is For the card name, only letters should be allowed and vice versa.
-
For the month field the user should only be allowed to enter numbers 1 to 12 as there are only 12 months, you can do a check for that.
-
The error Message for the Year field is overlapping that for the Month, try positioning them well.
7.) It will also be nice if when the user clicks confirm and there's any input field left blank, an error message will tell them that...
Hope this helps....
-
- @elaineleung@Aik-202
Hi Elaine, Nice work! You did justice to this!. The responsiveness is on point, but I have some suggestions: 1.) For the desktop site view on mobile... The text "Designed For The Future" overflowed to the next line.. try adjusting it so it takes up a single line. 2.) Then the sub headings "Introducing an exrensible editor" and "Robust...", Try increasing their font size and reducing that of their respective paragraphs... 3.) For Mobile normal screen the images seem too big, I advice you reduce them a little. And the drop down menu for Products, Company, Connect... Seems to have an issue displaying the contents appropriately... Try checking the JavaScript functions that activates that feature, I think that might be where the problem is coming from.
Marked as helpful - @codershivamkumarsingh@Aik-202
Hey Shivam, This is a Nice attempt. To answer your questions: 1.) It's not good to use margins to adjust the contents of your page. There are other ways you can do that. You can use CSS Flex, Grid. My personal recommendation is CSS Flex. So start by removing the margins from your div. then do this: div { display: flex justify-content: center; align-items: center; } display: flex allows you use CSS flex properties. Then justify-content and align-items set to center will completely center ya div container... Also set the width of your image to 100% and also use the max-width:100% property so you image takes the width of your container. To learn more about CSS flex, visit: https://www.w3schools.com/cssref/css3_pr_flex.asp
2.) The reason why everything is bigger on mobile is because your page contents are too big .. so the mobile phone is adjusting to the size of the page contents. I'll also advice you use Media queries . To learn about it visit : https://www.w3schools.com/css/css_rwd_mediaqueries.asp
I hope you find this helpful, keep improving on your solution!
- @Chromax-D
flexbox, grids, custom css and bootstrap
#accessibility#bootstrap#semantic-ui#vanilla-extract#webpack@Aik-202Hey, God's key... Nice work!! The responsiveness is on point!. But, the seem to be an issue with the buttons at the last paragraph of the page i.e "Clipboard for iOS and Mac OS" paragraph. For the desktop site mode, there's not enough space between the buttons. For the mobile, the seem to be too much space at the left-hand side, so the second button overflowed to the next line.
Marked as helpful - @Serg1oxD@Aik-202
Hi Sergio, Wow! You did amazing! What I loved most was the way the card details changed as the input of the user changes, nice idea. I just completed this challenge my self not quite long ago, I didn't think of that... I have some suggestions tho, the responsiveness is not quite there, I too battled with responsiveness. So here are my suggestions:
1.) For the desktop site view on mobile... Try reducing the width of the cards... And try using height: 100vh so that the background image will take the height of the entire viewport and you can also adjust the width of the image to give more room for the form.
2.) For Mobile responsiveness, try adding a media query for screen size max width 480px to your css and style accordingly.
Nice work so far!