Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Log in with GitHub
OverviewSolutions3Comments46

All comments

  • Alex• 660

    @al3xback

    Posted

    Hi Anosha,

    I think this structure is not well optimized when have long content. (https://prnt.sc/24lvdv1, https://prnt.sc/24lvvu0)

    Marked as helpful

    1
  • Alex• 660

    @al3xback

    Posted

    Hi Christopher,

    I think we dont need to add width 80% on .card class. Cos this will create empty space. Attached screenshot for a better view. (https://prnt.sc/24lspqh)

    0
  • Alex• 660

    @al3xback

    Posted

    Hi AleksandarV91,

    I see double scrollbar here, https://prnt.sc/24gxp6o

    Looks like there is a structure problem

    0
  • Alex• 660

    @al3xback

    Posted

    Hi Quincy,

    It would be good if we can set position: relative to each grid items, not in .Grid class. So this will lead to a better placement of the content.

    Marked as helpful

    0
  • Wildmarks Passos• 140

    @Wildmarks-Passos

    Submitted

    I noticed that in the original image of the challenge, in the color part -> #0c1729, there are some broken parts, does anyone know how I do this with CSS? I couldn't find anything relevant.

    HTML5, CSS3

    3

    Alex• 660

    @al3xback

    Posted

    Hi Wildmarks,

    I see that the color (#0c1729) is not working cos it's overridden by children's background-color (#15273f) on class contentContainer.

    The css concept is actually each children can overwrite/change the inherit value from the parent.

    Marked as helpful

    0
  • Alex• 660

    @al3xback

    Posted

    Hi Jane,

    When i tried to open on smaller desktop height, i could not scroll to see the overall content, looks like it's affected by overflow: hidden

    0
  • Alex• 660

    @al3xback

    Posted

    Hi Emiliano,

    For this case, it would be good if we use fixed value instead of percentage for max-width on container class. If we look on larger desktop or simply zoom out then we will see the difference.

    I would change to this:

    .container {
    max-width: 1200px;
    padding: 3% 5%;
    margin: 0 auto;
}

    Marked as helpful

    0
  • Alex• 660

    @al3xback

    Posted

    HI Mark,

    My feedback are as follows:

    • for class nft-photo, using flex properties is unnecessary cos we dont have content inside.
    • for class ETH and time, we dont need to use flex-direction: row as its default value for flex.
    • it's unnecessary to add color property on class line, it's alr handled by border.

    Marked as helpful

    0
  • Alex• 660

    @al3xback

    Posted

    Hi Achref,

    It would be good if we can move social and copyright to footer instead of section.

    Marked as helpful

    1
  • Alex• 660

    @al3xback

    Posted

    Hi Achref,

    By using place-content, behind the scene, we are adding special styles to grid-container (grid-template-columns: min-content; and grid-template-rows: min-content;) which make each grid-items shrinks so we can place them easily according to the value of place-content.

    Those additional styles won't work if we assign fraction unit value to grid-template-columns. Fraction unit provide grid-item the ability to occupy existing space (similar to grow in flex) and this will make grid-container difficult to manage position of grid-items.

    Hope this clear :)

    Marked as helpful

    1
  • Alex• 660

    @al3xback

    Posted

    Hi Achref,

    I think for this case we dont need to use place-items. This will break the layout if the grid-item doesn't have paragraph element. Attached screenshot for this case. (https://prnt.sc/23dredg)

    and for place-content, it doesnt have effect if we combine with fraction unit(fr) on grid-template-columns (cos usually only work for px or auto).

    Marked as helpful

    1
  • AliAbuhumra• 230

    @aliabuhumra

    Submitted

    I used the net for this challenge, I need your feedback please😊

    Social proof section

    #bem#sass/scss#semantic-ui

    1

    Alex• 660

    @al3xback

    Posted

    Hi AliAbuhumra,

    I would recommend using margin-top instead of translateY on .quotes__item, by using this will allows the parent's height equal to children height (including margin).

    0
  • Alex• 660

    @al3xback

    Posted

    Hi Natasya,

    My feedback is:

    • It would be better if we set image on header to be a background-image instead of img tag. The reason is:
    a) this is just static image and never changing too often (even no need for CMS setup for future).
b) we are not going to use picture element for better rendering.
c) less code.. haha
    • on main, changes will be:
    main {
    ...
    flex-wrap: wrap; //remove
    align-items: center; //remove
}

    we dont need align-items: center cos each .list element has width property 100%, then this has no effect :) and we dont need flex-wrap: wrap cos we dont set exact height to it.

    Marked as helpful

    0
  • Mohamed Abdessamed• 140

    @Brivan-26

    Submitted

    I really enjoyed solving this challenge where I applied the grid system, I used only 1 grid container, and I displayed sub-cards using the great command: grid-template-areas. I'm wondering if you could use breakpoints under 750px because I used 960px(under this view, the cards will be adjoining Your comments and criticisms are welcome

    Responsive Testimonials grid using only 1 grid container

    #accessibility

    3

    Alex• 660

    @al3xback

    Posted

    Hi Brivan,

    I think it is better to set grid-template-columns by using fraction unit (1fr). currently the grid items dont fully cover the grid container, leaving minus 1% on the right side.

    0
  • P
    Noemi Gali• 280

    @NomiDomi

    Submitted

    Hello guys,

    This is my second attempt to recreate this design. This was one of my first designs to implement before I was fully familiar with concepts like:

    • keeping your code DRY
    • using the right units
    • CSS variables

    Appreciate any feedback! :)

    Responsive mobile-first nft card component

    3

    Alex• 660

    @al3xback

    Posted

    Hi Noemi,

    It is better to change height to min-height on body. try to zoom-in until the browser height smaller than the content, and see the difference. min-height allows better view to see full content.

    Marked as helpful

    1
  • Alex• 660

    @al3xback

    Posted

    Hi Eshan,

    My feedback is:

    • step 1: remove place-content and background properties(except: background-color) from body and add some grid properties.
    body {
   ...
   place-content: center; //remove
   background-color: //keep!
   background-image: //remove
   background-position: //remove
   background-repeat: //remove
   grid-template-columns: 1fr auto 1fr; //add
   grid-template-rows: 1fr auto 1fr; //add
   overflow: hidden; //add
}
    • step 2: add 2 new html elements, place it before main, the structure will be like this.
    <body>
   <div class="decoration-bg decoration-bg-1"></div>
   <div class="decoration-bg decoration-bg-2"></div>
   <main>...</main>
   <footer>...</footer>
</body>
    • step 3: add styles to newly added elements.
    .decoration-bg {
    position: relative;
}

.decoration-bg-1 {
    grid-area: 1 / 1 / 2 / 2;
}

.decoration-bg-2 {
    grid-area: 3 / 3 / 4 / 4;
}

.decoration-bg-1::before,
.decoration-bg-2::before {
    content: "";
    position: absolute;
    width: 980px;
    height: 980px;
    background-size: 100% auto;
    background-repeat: no-repeat;
}

.decoration-bg-1::before {
    bottom: 0;
    right: 0;
    background-image: url(./images/bg-pattern-top.svg);
    transform: translate(20%, 20%);
}

.decoration-bg-2::before {
    top: 0;
    left: 0;
    background-image: url(./images/bg-pattern-bottom.svg);
    transform: translate(-20%, -20%);
}
    • step 4: add styles to main and footer.
    .card {
   ...
   grid-area: 2 / 2 / 3 / 3;
}

.attribution {
   ...
   padding: 12px 0;
   grid-area: 3 / 1 / 4 / span 3;
   align-self: end;
}

    Marked as helpful

    0
  • Mawaddatur Riza• 20

    @rizahoemae

    Submitted

    Hi, I'm just curious about the color of the text in the description. Is it okay if I use hsl white color(0, 0%, 100%) with 50% opacity instead of new color? I also coded both (mobile & desktop) in one style, is that okay? Thank you in advance

    NFT preview card component challenge hub

    #tailwind-css

    2

    Alex• 660

    @al3xback

    Posted

    Hi rizahoemae,

    In my opinion, using the opacity property to reduce the contrast of a color is not a good idea, because there is a condition where if in future we want to add an image, icon or other element inside that element, then the newly added element will also be affected by the opacity.

    It is better if we set through tailwind text-opacity:

    from:

    <div class="text-white opacity-50 text-[14px] mt-2">Our Equilibrium collection promotes balance and calm.</div>

    change to:

    <div class="text-white text-[14px] mt-2 text-opacity-50">Our Equilibrium collection promotes balance and calm.</div>

    This approach will only inherit color.

    1
  • Alex• 660

    @al3xback

    Posted

    Hi Quang,

    Would be good if we dont set width on each grid item (in this case on .card class) if we use grid-template-areas.

    let the parent controls the width of its children using grid-auto-columns: 1fr; or grid-template-columns: repeat(5, 1fr);

    Marked as helpful

    0
  • Alex• 660

    @al3xback

    Posted

    Hi Ryan,

    My feedback is:

    • use only one <h1> in a page.
    • adding width: auto and height: auto to .card__image is unnecessarily unless we use width and height attributes on img then neutralize it with css.

    Marked as helpful

    0
  • Alex• 660

    @al3xback

    Posted

    Hi Morden,

    Looks like in larger screen this needs to be improved.

    • step 1: set minimum height to body to cover at least the same as browser height and set background-size to fill it.
    body {
   ...
   min-height: 100vh;
   background-repeat: no-repeat;
   background-size: auto 100%;
}
    • step 2: for better html structure, let the <section> tag out of <header> tag and then wrap with <main> tag. The structure will be like this:
    <header>
   <nav>...</nav>
</header>
<main>
  <section>...</section>
</main>
<footer>...</footer>
    0
  • Alex• 660

    @al3xback

    Posted

    Hi Priscila,

    My feedback is:

    • try to add space between section.
    .small-content {
   ...
   padding: 60px 0;
}
    • for the shape bg, would be better if we set it behind the content by adding negative z-index.
    .shape-fe {
   ...
   z-index: -1;
}
    0
  • Alex• 660

    @al3xback

    Posted

    Hi Hamed,

    My feedback is:

    • when we use flex-basis on card-img class, it would be better if we add max-width.
    .card-img {
    ...
    max-width: 50%;
}

    Try to change img url to something larger image and see the difference.

    Marked as helpful

    0
  • Alex• 660

    @al3xback

    Posted

    Hi Natasya,

    For the arrow icon, it would be better if set to middle :) The problem is in the heading which still inherits the user agent properties esp margin.

    We can try something like this:

    • step 1: make sure each heading doesnt have margin, let the parent(article) use padding.
    article {
   ...
   padding: 1rem 0;
}
article header h4 {
   ...
   margin: 0;
}
    • step 2: slightly make the button in middle position.
    button {
   ...
   display: inline-flex;
   align-items: center;
}

    Marked as helpful

    1
  • Alex• 660

    @al3xback

    Posted

    Hi Natasya,

    Looks like for the header bg is not fully cover the entire screen, esc when viewed on larger device. Try to zoom out the browser and see.

    My feedback is:

    • add full width to .header__bg__image and inheritance to .header__bg__image img selector.
    .header__bg__image {
   ...
   width: 100%;
}

.header__bg__image img {
   ...
   width: inherit;
}
    • for All, Active, Completed, and Clear Completed would be good if we set it into button element.
    <button type="button">All</button>
<button type="button">Active</button>
<button type="button">Completed</button>
<button type="button">Clear Completed</button>

    Marked as helpful

    2