Skip to content
OverviewSolutions
13
Comments
6

Billy Hick

@TheArkhamKnight781526England330 points

Student Web Developer.

Latest solutions

View more solutions

Latest comments

  • Billy Hick•330
    @TheArkhamKnight781526

    Good work! A couple of issues I noticed:

    The padding isn't quite right for the form buttons/inputs. There's a space missing between the bold text and the regular text in the purple container. The font sizes aren't quite right.

    And one major issue: If the height of the page isn't big enough, the whole thing will disappear, and there's no way to scroll up. This is only an issue on desktop, though.

    Nevertheless, overall, good work! Hope this feedback was helpful!

    Marked as helpful
  • Billy Hick•330
    @TheArkhamKnight781526

    If you use put the price and the days in one div, you can use this snippet:

    #container {
    display: flex;
    justify-content: space-between;
    align-items: center;
}
    Marked as helpful
  • Billy Hick•330
    @TheArkhamKnight781526

    Overall, this is very good! I would try: Animating colour changes

    Making the cursor a text cursor when hovering over text (did you change this intentionally?)

    Saving the theme/user in LocalStorage/Cookies

    Unfortunately, I did notice one major bug - when you click on the link to the website, unless the link contains http:// or https:// at the beginning, it will not link to the external website. You might want to write some code to check for this and make sure that all links are redirecting properly. Otherwise, good work!

    Marked as helpful
  • Billy Hick•330
    @TheArkhamKnight781526

    Overall, great work! Just a few things I would reccommend: First of all, have the accordion expand downwards so your user doesn't need to move the mouse to close it again - like how hamburgers should stay in the same place whether the menu is open or closed. Secondly, (this is minor) maybe try animating it with CSS? It's a small thing but it makes the experience feel much smoother.

    Once again, well done!

  • Billy Hick•330
    @TheArkhamKnight781526

    Overall, great work! One thing, though: Your view icon only shows when you over over the specific area that the icon contains. Ideally, it would show when you hover over the image as a whole - you might want to change it to that instead, as initially it can look buggy and like the view icon isn't appearing correctly.

    Marked as helpful
  • Billy Hick•330
    @TheArkhamKnight781526

    1. The width of the window can be found using window.innerWidth. (For my version, I simply loaded in both images and then hid/showed them using CSS media queries.)

    2. I don't think animating a change of state is necessary, but a post can be found here about it: https://linguinecode.com/post/how-to-add-react-animation

    3. Not entirely sure what you mean.

    4. Don't worry, I struggled with that as well! However, the animation you've done unfortunately overlaps with the text sometimes, so I'd consider changing that.

    Other notes:

    Your mobile menu doesn't have the frosted glass effect in the design.

    I would add cursor: pointer to your mobile menu links.

    The lilac text colour isn't used on all of the elements it should be, and on the home page it only appears on the mobile design.

    There is no background for the navbar.

    You should add the same padding/margin to the side of the body as you do to the navbar, because otherwise it looks misaligned and also it doesn't look great when text is just squeezed up against the edge.

    On the crew page, when you go to desktop design, the switcher doesn't line up with the text.

    On the technology page, you need to add a vertical margin for the switcher so the buttons don't look all squashed up together on desktop.

    You might want to consider adding a small bit of padding underneath your mobile menu links, so the border is a little bit spaced out.

    There is no hover/active styling for the switcher on the crew page. Also, I would give each switcher button the same margin and not use justify-content to space them, as it gives a weird blowing up/deflating effect when you resize.

    The technology images do not span the whole page, unlike in the design.

    The text on the home page is not aligned.

    The divider on the destination page should be a border of the actual stats component, otherwise the stats go out of the bounds of the divider.

    The border is slightly too thick on the navbar, and because of the ::focus outline, you can see that the border and the outline are not the same.

    And finally... Well done!!!

    Marked as helpful