Skip to content
OverviewSolutions
23
Comments
9
Marta
@martam90

All comments

  • Marta•630
    @martam90Posted about 3 years ago

    Hi, Good job when it comes to logic of an app. The only thing I am missing is the website responsiveness.

    0
  • Marta•630
    @martam90Posted about 3 years ago

    Hi, I would have a look on those issues:

    1. I can't see the number of people in the input. I choose the number, but it won't display there.
    2. In JS I would add/remove classList instead of hard coding css styles.
    3. It seems to me that total amount and tip amount are calculated wrong. There is one more zero add to every amount. The tip for the bill of 100$ for 1 people should be 10$, instead I see 100$.

    Overall, I think you have done nice work :)

    Marked as helpful
    1
  • Anna Leigh•5,135
    @brasspetalsSubmitted almost 4 years ago

    Hi, everyone! 👋 I really enjoyed this one, probably because I like the design so much. Finally getting more comfortable with grid, and might even be starting to prefer it over flexbox!

    The Sass architecture is probably a bit overkill for a project this size, but I wanted to practice something more akin to 7-1 architecture. I also didn't implement mix-ins to this project, but plan on doing so in the next.

    Also, this solution uses 62.5% font-size. I’m aware of the issues with it and will try out other methods in future projects. 👍

    I really tried to stick to just 750px and 1200px for my main media queries. I was very tempted to add a third to switch a little earlier to the vertically-styled cards and the horizontal overlap style for the “interactive VR” section, since neither work at 750px. Ultimately, I figured it was a bit too much. Thoughts?

    I added a subtle image zoom on hover to the “Our Creations” cards using ::before pseudo elements. Is there a better and/or simpler way to go about this?

    I’m trying to implement better accessibility practices, so for this project I made sure to put the mobile menu button inside the nav and also added the aria-expanded attribute. Is this the correct way to go about this? Any suggestions on accessibility and how to improve it are very welcome!

    Any and all feedback is greatly appreciated! Thanks for taking the time to look at my solution. 😄

    Loopstudios - SCSS. Grid, Flexbox, JS, CSS animated mobile menu

    4
    Marta•630
    @martam90Posted almost 4 years ago

    Hi Anna,

    I have seen your solution (and others too) and I must say I really admire your pixel perfect solutions. :) I am going to code this challenge and I think your solution might be very inspiring. :)

    1
  • Sibasish Sinha•125
    @SibasishSinhaSubmitted almost 4 years ago

    I tried my best to make the page responsive as possible. I think it looks good and according to the design for 375px screen as well as for 1440px screen. The only problem I had, is to align the footer items according to the design given. I created the footer elements according my personal choice. Any kind of feedback and critics are more than welcome and accepted ! :D I just want to get better at CSS

    Clipboard Landing page with HTML & CSS

    2
    Marta•630
    @martam90Posted almost 4 years ago

    Hi, When it comes to footer, you can apply CSS Grid to all footer section. Within that CSS Grid I would use another CSS Grid and apply it to .footer-text.

    1
  • Marta•630
    @martam90Posted almost 4 years ago

    Hi,

    Nice work. Here are my suggestions:

    check your report - there are HTML issues with section and using headings.

    I think buttons in main should have bolder text when you hover on them.

    Set margin: 0 to body. You will remove that white space on the left side of the main.

    In mobile menu version menu-items should have lighter color. Check it with design.

    I would think how to improve intro-mobile-image. It doesnt look good for on devices that have 425 px (the image is in the center and on its left and right are blank spaces).

    I hope this might be helpful :)

    0
  • Marta•630
    @martam90Posted almost 4 years ago

    Hi,

    Here are my suggestions, I hope it will be helpful: Add hover state to buttons Add alt tag to all images in html - See Report generated in your solution Add a href="" tags to li in footer. Then you can add hover state to a element either. Add hover state to social icons and give some more space between them Logo in footer should be smaller. Try to add to svg viewBox https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/viewBox

    Overall nice work :)

    1
  • Marta•630
    @martam90Posted almost 4 years ago

    Hi,

    I have recently made the same challenge as you. Here are my suggestions:

    Add cursor: pointer to buttons; Add hover state to icons in footer-social. Instead of putting img in index.html, paste there svg code for each icon; Add alt attributes to all images in index.html; Add some line-height to <p> tags;

    Overall nice work, I hope you will find my hints helpful. :)

    2
  • Marta•630
    @martam90Posted almost 4 years ago

    Hi, your solution looks nice. I would suggest two things:

    • Add line-height to .answer, so the text will looks a little bit nicer,
    • You can try to hide the content of .answer by clicking on arrow up again. If you know JavaScript a bit, that might helps a lot but I think it is possible to make it in HTML/CSS as well.
    0
  • Marta•630
    @martam90Posted almost 4 years ago

    Hi, your solution looks good. I would apply on button hover state and cursor: pointer

    0