Skip to content
OverviewSolutions
15
Comments
17
P
Harm Intemann
@ghintema

All comments

  • P
    Harm Intemann•590
    @ghintema

    Hi Tereza,

    I've had a look at your solution to the galleria slideshow. I am really impressed!! :) I am trying to build it for myself. I am not sure, how to realize the slider. Should I make a real slider with Javascript? Or in this case bunch of different pages linked to each other??

    I'd like to ask you something and woud like to learn something from you. It's about this query-like url with 'painting=0' at the end. When you zapp your slides, it just increases this 'painting-parameter'. I don't know that technice and would like to learn more about it. As far as I understand it, you have only ONE page (html-file) for all the different slides and depending on this 'painting= ? ' parameter do you choose, what is beeing shown. Right? Can you give me a short explanation of how it works? Do you have a key-word for me to google this technice? Do you need a framework to do this trick?

    I highly thank you for your help!! :) Harm

  • P
    Harm Intemann•590
    @ghintema

    Hi Undie Fidelis,

    I just looked at your solution to the project. I have to admit that I don't know a shit about tailwind! :) Never used it and never even tried to. So hard for me to make concret tips on how to improve. But I think I could give you some kind of 'strategies' or advices on how to improve things in general. Here is what I'd do or did in my own solution respectively:

    1. on hovering the 'garbage-bin' delete-button in the cart: give it a transform: scale(1.3) to make it stand out and clearly mark it as click-able. The 'checkout' button as well misses a hover-state. You should do the same as with the 'add to cart' button.

    2. The underline when hovering the main-menu-items: Try to give the menu-items a padding-bottom, so that this padding is the lead in creating the space to the horizontal grey line bellow the head. And on hover you creat the border-bottom of the menu-items. You probably also have to set 'box-sizing' property to 'border-box'. This way you should get two things. First (and most important) The main-layout should stopps "jumping" when hovering the menu-items, because the appearing border-bottom doesn't take extra space! Second: The underline gets down to the grey horrizontal line according to the design.

    3. You should add a way to close the cart other than reloading the page :) One easy way could be: On clicking the checkout-button you return to page. Problem with that: It's not intuitive and with an empty card you don't even have it. So I integrated a 'cross' symbol in the upper right corner of the cart to clearly mark it as close-able. Also with transform: scale(1.3) on hover. The cross can easliy be generated with a span of innerHTML: × like this: <span class="closing-cross" id="close-cart" role="button" aria-label="close cart preview and return to page">×</span></p>

    As I said I didn't know how to integrate these things in your code of tailwind. But maybe I could inspire you a bit on how to improve things overall... If you like, have a look at my own solution of this project. Always happy coding and happy learging!! :)

    Harm

  • P
    Harm Intemann•590
    @ghintema

    Hi Laura,

    with respect to responsivnes I'd recommend you to give the credit card images (front and backside) a fixed(!) size so that the card-content (number and name) don't breake. If you do want to have it growing and shrinking, then try a min-width of 300px. Thats enough to prevent the number from breaking. Try to get the cards move to the left, if you run out of space on small desktops and/or reduce the whitespace between the form and the cards. In my own solution I gave the cards a position: absolute with left in %. The gap between form and left side also in %. You should better change to mobile-layout bevor you breake the card-number or get a vertical scrollbar or cover the form with the cards (wich may happen with a fixed card-size or min-width).

    I hope I could have helped you a bit and wish you all the best for your further learning. Me too, I'm probably just a little beyond beginner level. Best regards from Bolivia, Harm

  • P
    Harm Intemann•590
    @ghintema

    Other part of the problem could be the sizing of the cards wich depend on the ch... I'm not fully familiar with the clamp-function. But I would try to assign a fixed with or %-width to all cards to make sure that all cards have the same width indepedently of there content...

  • P
    Harm Intemann•590
    @ghintema

    Hi Daniel,

    with respect to your question, why are the two cards in the middle smaller than the outer two cards... You created an 8-column grid and assigned three columns each for the outer ones and two columns for the middle ones. So isn't it obvious that way to be different sized? :) This should fix the problem: Chance the grid-column for .card--supervisor from 1/4 -> 2/4 and for card--calculator from 6/9 -> 6/8. It worked for me in the dev-tool.

    Hope I could've helped you! :)

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi Elaine,

    did you notice, that when you reduce screen size below the 960px-Brakepoint you get to see the slide-menu for a sec?? You might think it's irrelevant as no real user plays along with dev-tools. But the thing is: You also see the effect when changing from landscape to portrait-mode. This can be annoying... No idea how to fix this, as I don't know nothing about JS so far. But I thought it's worth mentioning... Never mind if you already knew it!! :)

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi, I've just had a glance at your code and these are the things that rose my interest:

    • You should put your prices in an extra <p>-tag. It's more semantical.

    • there is an empty <div class="card-img"></div>. Supose you did that to give it a background-image. I think (and I'd suggest) its better standard to put an <img>-tag here. Espacially because it's a product-presentation and therefore semantical - and not only pure design. For desktop you can still exchange it anyway via css by setting the .card-img { content: url("") }. But this way you can add <img alt="description of picture"> wich improves accessibility...

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi,

    there are img not found... Do you know that? Further more: From semantical point of view its better to use <main> and <section> elements instead of only <div class="main-section">. Though technically this doesn't matter, of course... And yeah, its just a very small landingpage. But I think it's good to get used to the correct things early on... :)

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    I like your responisvness. Good job! All I would change is the growth of the download-buttons. You should limit its growth with a max-widht. It's irritating see them grow so large. :)

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi Dodeun,

    I've just reviewed your code and I've seen that you applied fix paddings on the sections. I'd suggest to change that to a % value to have more space for the content in case of smaller screens. You would improve responsiveness this way...

    And from semantic point of view: I think its better to make the "build the comunity..." - call to action part of the main. You've put it in the header. For a reason?

    Hope to have helped you. Always happy coding! :)

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi Marta,

    just reviewed your code and all I found is that there is a slight issue with the indentation :) Lines 20 to 30 need one more tab indentation. Right now, the div#box begins in the same column like the parent body. All the rest look neat and tidy to me! :)

    Hope to have helped you, keep going and happy coding! :)

  • P
    Harm Intemann•590
    @ghintema

    Reviewing your code I've seen that you have a double </main> at the end of your html. You should remove one. All the rest looks really good to me!! Clean and tidy code! :) I think you should remove the (max-width:1440px) on the second media-query to not exlude very large screens... Cheers, Harm

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi Shikhar,

    At first I did the color-bars on top of each card as border-top - just like you did. To prevent the these colorbars from curving at the end, I finally modeled them as an extra <div> of only 5px height and with border-radius:5px 5px 0px 0px; This way you get a color-bar without any curving left and right.

    To fix the icon position I propose you this way: Give the cards a position:relative and the icons inside the cards a position: absolute. This way you can easily fix the icons in the bottom right corner of each card using bottom:XXpx and right:XXpx; To "stablize" the entire layout I would also fix the width and height of each cards.

    Hope to have helped you a bit, Cheers, Harm

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi Lahiru,

    I reviewed your solution and would like to give you some feedback. What rises most my curiosity is this: Why building two different <nav></nav> areas in html?? You have one for desktop and an extra-one for the slidebar. Don't you think it would have been better using only one for both devices?

    When sliding the viewport from small to large to x-large, you put the extra available space in the center of the design, seperating the two main columns. I personally think it's better to keep the layout and item-distances constant and put extra white-space left and right of the design instead. What do you think about the idea? Furthermore, the different adaptions from mobile to desktop (for example change of picture, letter-sizing, layout-changes...) aren't really synchronized. For example at viewport=900px you see the mobile-one-column-layout in combination with the large desktop-picture. You could think that both of these things aren't really an issue, as you usually don't slide your viewports and you only use one fix device at a time. But things really get irritating (if not annoying) when you start zooming in and out of the page. That's when you notice all these things. My proposal: Make one complete change from mobile to tablet to desktop, once there is enough space available.

    I hope this feedback helps you! That's really all I want to do! If you have further questions on my comment, don't hesitate to ask them. And here is my solution to this challenge. Fell free to test and comment it as well. :) Cheers, Harm

    https://www.frontendmentor.io/solutions/introsectionwithdropdownnavigation-fully-css-no-js-qPsLKPlTKg

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi Hamid-Abdallah-Mohammed,

    I've reviewed your code and I'd like to give you some comments that may be useful to improve yourself. First of all I am wondering, why you've build to different navigations in the html-structure. You have one for the desktop-version nested into <nav></nav> (wich is good, of course) and a second navigation nested into <div class="mobile-nav"></div>. This irritated me a lot! :) You really could (and should) have done mobil and desktop with the one nested in the <nav></nav>. Secondly I would advice you to always build your menu-structure in list-elements like this: <ul><li></li></ul>. You can even do that with two or more layers of menu-structure by nesting a <ul><li></li></ul> INSIDE A <li></li>. It really is more semantical and (much) easier to layout one single <nav></nav> for mobile and desktop. One last (miner) thing that rose my curiosity is this (from line148): grid-template-columns: repeat(1, max(200px, 350px)); What sense does it have to max(250px, 350px)? The max of 250 and 350 is always and forever 350px.

    If you have further questions on my coment, don't hesitate to get back on me! I hope I could help you a bit. Really mean to do that... Cheers, Harm

    Marked as helpful
  • P
    Harm Intemann•590
    @ghintema

    Hi Ayjamal,

    thanks for the advice. Really looks good! I completely missed that shadow in the original design and in the figma-files.

  • P
    Harm Intemann•590
    @ghintema

    Hi Gavin,

    thank you for your feedback. It's highly appreciated. I really thank you for it. And after a while of thinking and trying I even got it WHY the z-index is the key here: The button is otherwise covered by a none visible area of the logo, so it's been visible but not hoverable.