@Top-Trekx-Im-gvp-98Submitted 9 days ago
ItsZubek
@ItsZubekAll comments
- @ItsZubekPosted 9 days ago
The javascript part looks alright, however when the dismiss button is pressed the signup form re-appears but the sucess message stays where it was. Another thing is that whatever email I put in it still shows the default one at the sucess screen, it doesn't update automatically to reflect what email has been used in the form.
As for the CSS I see that you've ignored the mobile design completely. It is really important to practise both the desktop and mobile design and to think about it when creating the html layout to accomodate both cases if necessary. Some styles are off, for example the success message is not styled at all and looks very different from the design.
Overall it is a good project, but I would spend a little more time on getting the little things right as it will look better on your github.
0 - @yosefshawahSubmitted 9 days agoWhat specific areas of your project would you like help with?
hello y'all, can someone point me to repo/guide how to do the shadow-box correctly and managing fonts, thanks!.
@ItsZubekPosted 9 days agohere you go: https://box-shadow.dev/ and https://cssgenerator.pl/box-shadow-generator/
The latter one works better for me, but it's in Polish
1 - @ttwmfSubmitted 10 days ago@ItsZubekPosted 9 days ago
I like the solution as it is working as intended. But it would be best if you created the mobile layout as well as it is important for all work to be usable on mobile devices - not just for the challenge but in real life situations as well. Other than that no other issues apart from some colour differences to the design, but that's not a real issue.
0 - @AuroreTurtleSubmitted 12 days ago@ItsZubekPosted 12 days ago
The cards themselves look very similar to the design. However I'm not sure if it's just on my monitor, but the horizontal spacing between the cards is completely off. I think if you've tried centering the whole design and adding a
gapproperty if you're using flexbox to space them out it would be okay. Another thing I've noticed is if you change the screen size to around700pxyou can see the background colours coming out from beneath the card itself. Other than that it looks good.0 - @hazhir00Submitted 20 days agoWhat specific areas of your project would you like help with?
I need help to make this design responsive
@ItsZubekPosted 20 days agoThe solution looks alright, however I see that the image is slightly smaller in width than the right side of the product card. You can fix it by assigning
flex-basisproperty a value of50%to both the image and the right side of the card. This way they will always take 50% of available space and basically, will be equal in size.As for the responsiveness of the design I see that you included
mediaquerries in your code, but it would be more effective if you set them a bit higher, i.e.600pxinstead of 400 something. I understand this is a rather small thing, but can still make your design look better.I see that you've used
imginstead of thepicturethat was discussed in the article before this challenge. This forces you to usedisplay: nonewhen changing the screen size to mobile from desktop and vice versa. Checkpictureonline as it will make next mobile responsive designs a lot easier for you and you will avoid unnecessary code.Also
alttags are important for accessibility. I understand this is just a challenge on a frontend website, but it's good practice to not forget them even when working on fake projects as it will create a possibly harming habit in the future.Overall the design and code look good, but some small tweaks can be made to make it more readable adn resemble the design a little more.
Marked as helpful1 - @GProostSubmitted 30 days ago@ItsZubekPosted 22 days ago
All necessary HTML is there. However I think you placed the
tdandthwrong way around.this table header which should be the first column of the table (calories, carbs, etc. ) andtdis table data which are the values. I'd improve on the CSS aspect of the project as it seems you gave up on it halfway through. I can clearly see you know how to style the elements, but you didn't choose to do so in the later part of the design. As for the table an easy fix to make it look more like the design is to use thewidthproperty and set it to100%and then select thethandtdand make them 50% and it should sort out the issue. Overall a good attempt and solid work, but I would try to finish the challenge even if you 100% know how exactly to do it - practice makes perfect.0 - @tobaojoSubmitted 23 days agoWhat are you most proud of, and what would you do differently next time?
How the solution came out considering I had to eyeball the designs without using a figma document.
What challenges did you encounter, and how did you overcome them?No figma file (may subscribe).
Lots of trial and error to get the design to a place I am happy with without any defined design styles
What specific areas of your project would you like help with?Just any insights or anything spotted that isn't like the designed document from my solution.
@ItsZubekPosted 23 days agoThe design looks good, although a little more testing would be nice to get it to look closer to the design, but overall is good.
As for the coding part, the HTML file looks good, as for the css - I have to point out that using fixed values (
px) for images might not be the best idea because of the responsive aspect of the component. If you'd just set the width, the height would adjust automatically. And usingremwould make the design more versatile.Same goes for fonts, using
pxmight not be the best idea - usereminstead.When it comes to the list part of the code the only thing I would point out is again, setting the
widthproperty to a fixed value. What would happen if you made the text within that list longer? Try setting thewidthto 100% and instead of usingheightusepaddingto make it the right size. If you were to change thefont-sizeproperty the height would remain the same and you'd need to adjust it manually. Using padding makes the button scale automatically to thefont-sizeproperty.Marked as helpful0 - @ZugimSubmitted 26 days agoWhat are you most proud of, and what would you do differently next time?
I was pleased that I was able to implement fonts responsively using
What challenges did you encounter, and how did you overcome them?remunits and media queries. When the viewport's width is less than 300px the font size for the component will decrease slightly. While not really necessary for this particular project it will hopefully come in handy for future projects.I encountered some weirdness when styling my header, which is a link nested inside a h2 element. I was originally trying to style the h2 element but the font wasn't displaying as expected. I did some research and discovered I should instead style the nested element.
What specific areas of your project would you like help with?I was wondering if people usually use
rem/emunits project wide or is it primarily for fonts? I was thinking they could also be useful for padding and margins.@ItsZubekPosted 25 days agoGood use of semantic HTML and CSS variables. Layout looks good. Solution is improved upon from the original design by adding 2 extra fields to the category section. Good use of css grid property.
0 - @jedcancholaSubmitted 26 days agoWhat are you most proud of, and what would you do differently next time?
I was able to organize my CSS so I can reuse some variables and write the less style as possible. Additionally I used semantic HTML
What challenges did you encounter, and how did you overcome them?NA
@ItsZubekPosted 26 days agoThe HTML looks good, it is accessible and no further improvement is needed. However since it's a really simple, entry-level project with a small amount of styles I feel like adding the css file might be a little unnecessary for the amount of code required to complete the challenge. However overall the solution is what it's supposed to be. Solid work.
0
