Skip to content
OverviewSolutions
30
Comments
27
P

Eli Silk

@elisilkBrookline, MA, USA850 points

I enjoy putting together websites. It's that simple. I love the structure, the beauty, the interactivity, the display and sharing of information, and seeing the concrete results of the work right there on the page in front of you. I'm learning new things everyday, and I'm fascinated by it all.

Latest solutions

View more solutions

Latest comments

  • P
    Eli Silk•850
    @elisilkPosted 3 months ago

    Hi @techmeg,

    Congrats on an excellent solution. 👏

    I realize you submitted this solution 4 years ago, and so you likely have moved on. But if you are up for diving back in 🤿, here are a things to consider:

    Like you, I also followed along with Kevin's solution on Scrimba. And I also learned a lot from his approach, while still having some differences from him in my own preferred approach. I wrote up some of the differences and changes that I made in my repo's README:

    https://github.com/elisilk/space-tourism-website?tab=readme-ov-file#a-strong-foundation

    Also like you, I was particularly frustrated about his JS approach not working for me, which I tracked down to an issue about display: block having precedence over the hidden HTML attribute. I believe Kevin's solution only worked because he mistakenly misspelled "picutre" in his CSS reset, and so the display: block wasn't actually applied to his <picture> elements. In the end, correcting the spelling of "picture" and then adding the following to the CSS solved the issue for me:

    [hidden] {
  display: none;
}

    Another area that I diverged from Kevin's solution and that I think would help improve the UX for your solution is in the main navigation. Specifically, I think it's important to increase the clickable area around those links and there's a lot of block padding to work with around them. More concretely, I think it would be best to move the block padding to the <a> elements rather than their parent <li> elements.

    Anyway, just some ideas to think back on if you want to revisit this. 🤔

    Great job overall on your solution! Happy coding. 💻

    Eli (@elisilk)

    0
  • P
    Eli Silk•850
    @elisilkPosted 3 months ago

    Hi 👋 @Sandaruwan7056,

    Congrats on another excellent solution. 👏

    I really love how you implemented transitions with the changing of colors and the scaling on hover of the articles in the extension area section. I think the transitions were very nicely done.

    If you are up for diving in 🤿 a little further, here are a couple small things I noticed:

    • There were a couple of places where I noticed that the letter spacing of the text could be adjusted to better match the design. For example, the navigation links and the "35,000+ already joined" in the footer both have fairly large letter spacing in the design (1.5-2.31px and 5px, respectively). In Tailwind, you can use "arbitrary values" to set those to be larger than what is set as the default on the tracking-widest setting. Or "customizing your theme" is another good option.

    • In your Features section, I'd suggest making aria-selected="true" by default on the first "Simple bookmarking" link in the features section so that that first button shows the red active bar when the page is first loaded.

    Anyway, just some ideas to consider if you are thinking about improving on your solution. 🤔

    Great job overall! Happy coding. 💻

    Eli (@elisilk)

    Marked as helpful
    0
  • P
    Nikita Vologdin•710
    @NikitaVologdinSubmitted 4 months ago
    What are you most proud of, and what would you do differently next time?

    Responsive and accessible Room homepage layout with optimized fonts and images. React components:

    • Animated slider
    • Dialog
    • Slider controls
    • Dynamic article

    React Room homepage

    #accessibility#framer-motion#react#sass/scss
    2
    P
    Eli Silk•850
    @elisilkPosted 4 months ago

    Hi 👋 @NikitaVologdin,

    Congrats on an excellent solution. 👏

    I really love the animations/transitions you've implemented. I'm especially impressed with how you have a separate animation for the photo versus the associated text. I think that looks really nice. And the animation of the mobile menu is well done too.

    If you are up for diving in 🤿 a little further, here are a couple small things I noticed:

    First, in the mobile viewport (375px wide), it looks to me like the flex container for the mobile popup menu is too crowded and that is causing the svg close icon to get smushed to 0px and also for there to be a horizontal scroll for the navigation menu. One option would be to lessen the gap property for the .nav__list class and that would solve the issue.

    Second, looking at the lighthouse metrics for your solution, I see that in the "Best Practices" section there is a note about "Displays images with incorrect aspect ratio". Looking more closely at the design, those two images in the bottom row don't have the same aspect ratio, and so they take up slightly different widths in the desktop view (420px vs 440px) and slightly different heights in the mobile view (238px vs 227px). To start, I'd suggest changing the HTML attributes of both images to be their intrinsic sizes:

    <img src="/images/image-about-dark.webp"
  class="content__image content__image_dark"
  width="420" height="266"
  alt="" loading="lazy">

<img src="/images/image-about-light.webp"
  class="content__image content__image_light"
  width="440" height="266"
  alt="" loading="lazy">

    The image will still be responsive as those intrinsic sizes will set up the correct aspect ratio for each image. Then you can set the CSS so that the images fill their container and calculate the appropriate height based on their intrinsic aspect ratios. So for .content__image, something like:

    .content__image {
  width: 100%;
  height: auto;
}

    Here is a great article for some background reading 📚:

    Setting Height And Width On Images Is Important Again by Barry Pollard on Smashing Magazine

    Then thinking of the .content flex container, that part can get a little tricky and there is not one best way. How you ultimately solve it is probably based a lot on how you want the container to change as you move from the narrower viewports to the wider ones. So there are different possibilities.

    Anyway, just some ideas to consider if you are thinking about improving on your solution. 🤔

    Great job overall! Happy coding. 💻

    Eli (@elisilk)

    Marked as helpful
    0
  • P
    Eli Silk•850
    @elisilkPosted 5 months ago

    Hi 👋 @Sandaruwan7056,

    Congrats on a very nice solution. 👏 It looks like you implemented all the key aspects of the challenge super well, especially as the viewport shifts seamlessly between the mobile and desktop views.

    I also really like how you implemented the transitions on the nav links. And I appreciate how your mobile navigation menu is fixed even when scrolled. Although both subtle things, I think they really help elevate the quality of the solution. Nice job.

    If you are up for diving in 🤿 a little further, here are a couple things I noticed:

    • In the design, there is subtle overlay on the "Our Creations" card images (even before the hover overlay). It's essentially a linear gradient that goes from a fully transparent black to a 60% transparent black. For the desktop view, it's positioned starting at about 30% down on the image and going to the bottom, and for the mobile view, it's positioned on the far left of the image and going to the right. That little bit of darkening allows the white text to pop a little bit so it's more readable.

    • I am learning Sass myself now, so forgive me if I get something wrong. But I noticed how in your footer partial that the social links are extending your regular footer links. It seems to me like the links in the hero nav share a lot of the same properties (especially the hover effect). Maybe that's an opportunity to continue to cut down on any duplication of code by having those links also be an extension of a more basic link class?

    Anyway, just some ideas to consider if you are thinking about improving on your solution. 🤔

    Great job overall! Happy coding. 💻

    Eli

    0
  • P
    Kamran Kiani•2,760
    @kaamiikSubmitted 6 months ago
    What are you most proud of, and what would you do differently next time?

    I began a new learning path on Frontend Mentor, and this was the first challenge I completed. I chose the CUBE CSS methodology for styling my HTML code.

    I don't plan on repeating this challenge.

    What challenges did you encounter, and how did you overcome them?

    One of the challenges was deciding whether an interactive element should be an a or a button. I chose the button because it doesn't navigate to a new page but performs an action. For the hover style, I used ::before and ::after pseudo-elements to add a background color and an icon image to the clickable button.

    What specific areas of your project would you like help with?

    I’d appreciate feedback on the CSS code and methodology I’ve used, specifically if it's been applied correctly. Additionally, if you notice any issues with my HTML code or accessibility, please let me know. Your feedback is incredibly valuable to me.

    NFT Preview Card with Astro, Sass and CUBE CSS

    #accessibility#astro#cube-css#sass/scss
    1
    P
    Eli Silk•850
    @elisilkPosted 5 months ago

    Hi 👋 @kaamiik,

    Congrats on a very nice solution. 👏

    I really like how you tried to use the CUBE CSS methodology, as I also just completed this same challenge with the same goal. It was a little counterintuitive to me, but it looks like you were able to implement it well.

    If you are up for diving in 🤿 a little further, here are a couple things I noticed:

    • I think you can try combining the pseudo-elements for the hover overlay effect on the image into one ::before pseudo-element. That way you don't need to worry about z-index at all. You can also put the opacity directly onto the color using the relative value syntax of hsl(), which is one of my favorite features. This way the opacity applies only to the color, and is not also on the SVG image. In the end, the CSS would be something like this:
    .card__imgButton::before {
  border-radius: 8px;
  background-color: hsl(from var(--primary-400) h s l / 50.3%);
  background-image: url(/assets/images/icon-view.svg);
  background-repeat: no-repeat;
  background-position: center;
  opacity: 0;
}

.card__imgButton:is(:hover,:focus)::before {
  opacity: 1;
}

    • I'd also suggest taking off the width and height from the .card__img class. The image will resize to fill it's container, so those aren't really needed. And when you keep them in there, the image can get distorted or not fill up the full width of the card as the browser window is resized.

    • Finally, I think the font sizes in your final .card__profile section could be cleaned up a bit. It looks like the link to the name is a font size that is too large, while the font size for the text "Creation of" is too small. In the design, all of the text in that section is one, consistent font size, and it actually goes from 15px in the mobile version to 16px in the desktop version. For simplicity, I'd suggest making them all one font size, and so using your .fs-400 class would work.

    Anyway, just some ideas to consider if you are thinking about improving on your solution. 🤔

    Great job overall! Happy coding. 💻

    Eli

    0
  • P
    Eli Silk•850
    @elisilkPosted 5 months ago

    Hi 👋 @GiovanniMacario,

    Congrats on a very nice solution. 👏

    One quick thing I would suggest is to fix what happens when a use clicks on a board space. Right now, when a user hovers of one of the board spaces, the icon outline comes up, which is great. But then when the user clicks on the board space on top of the icon outline, the button never receives the button click. It only works when the user clicks on the blank space around the icon (but still within the button).

    I'd suggest adding pointer-events: none; to your .iconXgridhover and .iconOgridhover classes. What that does is "the value none instructs the pointer event to go "through" the element and target whatever is "underneath" that element instead" (Reference: https://developer.mozilla.org/en-US/docs/Web/CSS/pointer-events).

    Great job overall! Happy coding. 💻

    Eli

    0
View more comments