I'd like to read your feedback to increase my skills.
Lorenzo Franco
@LfrancosAll comments
- @HualDevSubmitted over 2 years ago@LfrancosPosted over 2 years ago
HualDev this is looking really good!
There are a couple of things that I think you can work on to improve what you have.
-
Right now you are not using the fonts that you are adding in your html. I would add to the body "font-family: 'Manrope', sans-serif; "
-
Also I feel the container is stretching a little more than the design shows, not that yours looks bad, but if you want to make it look a little more like the design you can add a "max-width: 540px" this will help to make sure the container will not be bigger than that.
-
I would make sure the width and the height of the Circle is the same. That way you will know that it is a perfect circle. and you add also "position: relative" (this is so you can center the dice) With that then you can add to the dice img: "position: absolute; top: 50%; left: 50%; transform: translate(-50%, -50%); "
Hopefully this is helpful : ) Keep the GREAT work!
Marked as helpful0 -
- @shubhsaurSubmitted over 2 years ago@LfrancosPosted over 2 years ago
Hello Shubham!
This is looking really good. The solutions that you found form some things are really inspiring.
There are a couple of things that I would try to do.
-
I would try to have 2 messages, one for when the email is not valid, and one when the field is empty. I think that would give clearer information to the user.
-
if I add an email I should be able to submit the form. to fix that you could take the "e.preventDefault()" and put it with in the if. That way you'll make sure that it is functioning with the "if" statement.
-
the button of the submit form when resizing the page should stay in the right position. First I would change the width of the email-input to "100%". Then I would change the "right: 1.2rem" to "right: 0" of the "arrow-btn"
Other than that I think this is looking really good! Congratulations and keep going with the AWESOME work : )
1 -
- @Wamiti8711Submitted almost 3 years ago
Trying to learn javascript form validation..Kindly, can anyone please join me as i learn form validation as a way of making the topic easier
@LfrancosPosted almost 3 years agoHello Joseph!
This is looking really nice!
There are 3 things that I'm seeing that can be improved.
-The first one is that the email input is accepting values that are not emails. You can take a look at my solution an see what I did to fix that.
- I think this is an important one and it is that the form is not submitting once all the values are accepted.
-And lastly is the responsiveness of the site. I highly encourage you to put max-widths to the more important elements. I'm saying this because your form is not keeping the same width as the price card. You added a "max-width: 500px" to the form but the pricing card not so it gets bigger and looks weird when it can grow more that that. I would suggest to add that max-width to the "form-container" that way you can make sure that all the items with-in them will keep the same width.
Hope this makes sense and helps you : )
0 - @AlexNixxSubmitted almost 3 years ago@LfrancosPosted almost 3 years ago
Hi Alex!
This is looking really nice!
I only have one comment and it is about the responsiveness of the site. Both the mobile and the desktop versions look really nice, but if you play around with the width of the screen you will see that there are point where the design brakes and and is hard to understand an read. I would recommend to see where is the optimal place to make the media query. Looking at your site I feel like the best place to make the brake is 1100px. That is going to make it stretch your main element a lot when it is below 1100px, but you can add a "max-width" to your main element. You can decide based on your taste what is the max-width that element.
Hopefully this makes sense and is helpful :)
Marked as helpful1 - @StarisblackSubmitted almost 3 years ago
i'm not really satisfied with the Javascript coding... any advice and suggestion is welcome. Thanks
@LfrancosPosted almost 3 years agoHi Samuel! This is looking really nice, Congratulations!
There are 2 things that are calling my attention in my perspective, but what you have is looking really nice.
- It would be nice that if you have errors it shows you all at once. Right now if I'm missing the name it tells me but is not showing any error in the other inputs. So I fix that one error and then I get another error. I feel that could be a little annoying for the user.
To fix that I would remove the return form the functions since every time the code finds a return it exits that function. You could add a variable that has the value "true" or "false" instead.
-
Would be nice if in the email input when it is empty it says "Email can not be empty" instead of "Looks like this is not an email".
-
And the last thing is that when you look at the code in 4k monitors the margin is to small and makes the design look a little to stretched, I would add a max-width to the main component making sure it will have an easy read for all the screens. Surfing the web I've seen that they recommend that the maximum width is 1440. But I encourage you to research and make the decision based on what you feel is right!
Hope this helps : )
1 - @Akinyemi4Submitted almost 3 years ago
Kindly help me check this out, I love corrections and better solution approach. Thanks
@LfrancosPosted almost 3 years agoHello Akinyemi!
This is looking really good, congratulations!!
There is something that is happening and it used to happened to me as well and it is that if you scale your screen up (meaning changing the height of the screen) you will see that half of the card is missing and even if I try to scroll up I'm not able to see what you have there. I encourage you to use in your body element "min-heigth: 100vh" instead of what you have that is "height: calc(100vh - 1px);". You will see that this is going to help you with that problem.
The last thing is that for accessibility you should add an element that has meaning for the the search engine. For this exercise I think you could use the element "main" that way the search engine would know that this is the main content and will know what your website is about.
Hope this helps : )
0 - @Clay8288Submitted almost 3 years ago@LfrancosPosted almost 3 years ago
Hello Clay!
This is looking really nice! I was trying to do what you did of making sure that when they click the share button the share pop-up would also close, but I couldn't do it : ( I'll take a look at your script to see what you did there!
The only thing that I feel is not working is that when you play around with the screen size some text gets out of the card and it gets really hard to read what it says. I think the main problem is that you are adding a set "height" I highly encourage you not to use that since that is going to bring you a lot of problems as the ones that it is causing for this website. Take that out and I'm sure you website will be responsive : )
Hope this helps !
Marked as helpful0 - @lukakavtarraSubmitted almost 3 years ago@LfrancosPosted almost 3 years ago
Hello Luka!
This is looking good! I can see that your report says that you have no problems, that is really good. Mine told me that I had one accessibility problem I need to fix that. I need to get in to the habit of of making sure that I'm not making any mistakes there.
The thing that I would encourage you to take more time is that you look if your website is working in responsive mode. I can see that they work pretty good in the resolutions that they provide, but if you play around with the size of the screen you will see that at certain points it brakes. The way that I've been approaching this is by working mobile first and I think it is really helpful for responsiveness.
One thing that I would do in your code is to add a min-width to the div that is below the main element. that way you can make sure that the height does not stretches as much as it is doing here.
But the thing that is making your responsiveness not to work is that you are adding a margin of -1.5rem to the "footer-flex" I would encourage you to make sure that you don't use negative numbers in there since that will make other elements go on top of that.
Hopefully this helps : )
Marked as helpful2 - @sz7kowSubmitted almost 3 years ago@LfrancosPosted almost 3 years ago
Just WOW! You did an incredible work with this! So many thing to learn form this.
1 - @TheBigDadOoSubmitted almost 3 years ago
I don't know why but I wasn't able to use :hover on the social medias links! Any ideas?
@LfrancosPosted almost 3 years agoHello David! This is looking really nice!! I tried to do the same thing you did with the footer of using flex-box but for some reason I was not able to pull it of. I ended up using grid and it worked but not as good as yours.
In you mobile version the footer has no space to the sides and it makes it hard to read when it is completely close to the borders. I would use some margin or padding to make sure that it has a little space to the sides.
Another thing that I'm seeing is that when you take a look at this in a 4K screen the whole design stretches too much. I would suggest to create a limit for the body. Looking around it seems that most of the people keep the content with in "1440px" I highly encourage you to keep that in mind so that your users don't have to move their heads too much side to side : )
Other than that this is looking INCREDIBLE!! Keep up the great work! : )
0 - @copocanetaSubmitted almost 3 years ago@LfrancosPosted almost 3 years ago
Hello Thiago! This is looking really GREAT! You where able to handle everything incredibly, I was not able to see that the background image was not supposed to repeat. The only thing that I'm seeing is that the social media icons are not supposed to go beyond the limit of the black background. From the things that I was able to see is that you are setting the footer a set "width" that way is going to be harder for the footer to be responsive. Also when I took the width out, I realized that you have some padding in there too, if you remove both of those you will see that the footer will stay with in the body limits, and will look more like the design!
This is looking really incredible! Keep up the incredible work! : )
Marked as helpful1 - @Luxeli0004Submitted almost 3 years ago
Constructive feedback is always welcome :) I was struggling a bit to create the design of the form because the "request access" button is inside the "email" input form. What would be the best solution to solve that?
@LfrancosPosted almost 3 years agoHi Elisabeth!! This challenge is looking really nice! The is something that is happening in your project that I would highly recommend to take a look. The problem is happening in you tablet and desktop views. if you push up the size of the window up (meaning reducing the height of the window) you will see that you start to lose some elements and is not easy to understand the website.
I'm new to web development so maybe what I'm about to say is really far from the answer, but looking at your code I feel like the thing that is causing that to happen is that you are using a lot of "position: absolute" from the things that I have learned to keep the website responsive you should only use that if absolutely necessary.
Other than that it is looking incredible! I haven't been able to make the error look like it is supposed in the challenge : ( will keep trying until I figure it out!
Keep up the good work!!
Marked as helpful0