Skip to content
OverviewSolutions
44
Comments
12
Hana
@Hanka8

All comments

  • Maca•630
    @Maacaa0Submitted over 2 years ago

    Quite an interesting challenge.

    Struggled with the shape of comment box but it worked eventually.

    Any suggestions are welcome.

    fylo-data-storage-component-master

    1
    Hana•960
    @Hanka8Posted over 2 years ago

    Hi Maca, nice job. Take a look at calc() CSS function to make calculation from px to em/rem easier.

    Marked as helpful
    1
  • Hana•960
    @Hanka8Submitted over 2 years ago

    Hi FM community,

    I dont understand why my z-index doesnt work on .mobile__content__answer. Other things seem ok for me. Any suggestions welcome 🦾

    Responsive page using pure CSS

    2
    Hana•960
    @Hanka8Posted over 2 years ago

    Hi Huynh, thanks for your comment. I added position: relative / absolute to my elements, but it still doestn work. I dont understand why (Its better seen when you change box-shadow´s color to for example red).

    .mobile__content__answer {
  background: white;
  color: var(--mobileTextColor2);
  font-size: var(--mobTextSize);
  line-height: var(--mobTextLize);
  border-radius: 10px;
  padding: 6px 8px;
  border-bottom-right-radius: 4px;
  align-self: flex-end;
  max-width: calc(128rem/16);
  position: relative;
  z-index: 2;
}

.mobile__content__answer::before {
  content:"";
  position: absolute;
  width:100%;
  right: 0px;
  bottom:-2px;
  transform:scale(.8);
  box-shadow: 0px 0px 5px 3px #eceaee;
  z-index: 1;
}
    0
  • Maca•630
    @Maacaa0Submitted over 2 years ago

    I thought it would be easier :D. Had trouble making it responsive, but at last, it worked. I would really appreciate feedback on this one. Thanks guys

    single-price-grid-component-master

    2
    Hana•960
    @Hanka8Posted over 2 years ago

    Hi Maca, nice job!

    I like your precise design. There is just one little thing i found in your code:

    • When defining heights and widths of the containers, its better to use relative units than px. Thats because when the user changes the font size in his browser, cards containing the text should grow with it to prevent overflow. All font-sizes should be also defined by relative units, because when you use pixels, its not possible to change the font size in browser. On the other side, paddings should be defined by px, because you dont want them to grow with the text. You can try changing the font size in the browser to see what it does to your layout. I suggest reading this article about units. You can also consider using rems instead of px in your media.

    • I noticed tha you are using ´display: flex´ together with ´width´. That can sometimes cause some confusions, consider using ´flex-basis´ instead. You can read about it here

    Keep up the good work ☺

    Marked as helpful
    1
  • Bence Zámbó•510
    @zambobenceSubmitted over 2 years ago

    I found it difficult at the beginning to wire together the JS part with the form answers, and at the end to figure out the different colours one might use for the backgrounds.

    I would appreciate any feedback on the css and JS part because I have a feeling that I have overcomplicated it.

    Thank you.

    Interactive rating component

    2
    Hana•960
    @Hanka8Posted over 2 years ago

    Hi, nice job!

    Here are some suggestions:

    • the color of the background is #1f2630 and buttons have #262f38, you should find the colors at the provided file style-guide.md, or if you are using firefox - there is a built-in dropper feature;

    • use cursor: pointer and add some transition effects on your buttons and labels;

    • you should edit cards padding and width according to the design, i suggest using box-sizing: border-box for it, because thats how you can use padding that is not affecting your cards width;

    • its better to use <footer> instead of <div> for your attribution as it is more semantic;

    • <label> should not have a name, the name attribute should be just in the <input>;

    • I used buttons instead of inputs and put the submit cards html at the dom, then changed the state from display: none to display: block (and vice versa) by clicking the submit button - this is just the different option, you can check out my solution if you want;

    Keep up the good work!

    Marked as helpful
    0
  • Hana•960
    @Hanka8Posted over 2 years ago

    Hi, nice job!

    I noticed just some details:

    • You should add same border-radius to your .image-overlay as your .image-top has;

    • style your hr color and width regarding the design;

    • you should add some box-shadow or use filter: drop-shadow to your card;

    Keep up the good work!

    1
  • Hana•960
    @Hanka8Posted over 2 years ago

    Hi txosca.

    You provided a creative solution for hovering the image. However, I think its better solution to use pseudo-elements ::before and ::after. This way the hover effect will always appear over the image. You can check out my codepen to study ::before/::after here. Try adding the class for the div containing the image and use something like the code below.

    Also I dont think its a good idea to mix in-line and external css in one project. It creates a big mess.

    I hope this helps!

    .div-image {
  position: relative;
}

.div-image:hover::after {
  content:"";
  position: absolute;
  inset: 0;
  border-radius: 15px;
  background-color: hsla(178, 100%, 50%, 0.7);
}

.div-image:hover::before {
  content:"";
  position: absolute;
  inset: 0;
  background-image: url(images/icon-view.svg);
  background-repeat: no-repeat;
  background-position: center;
  z-index: 100;
}
    Marked as helpful
    1
  • leverh•300
    @leverhSubmitted over 2 years ago

    Fixed solution. Please note the git repository url is for old version. Could not update new files onto git as hard as i tried... The live site URL however is the correct version.

    Stats preview card component

    1
    Hana•960
    @Hanka8Posted over 2 years ago

    Hi leverh, nice job!

    You wrongly placed the end of the </div> in your html (you meant it under the attribution?) - I think its the root of your problems.

    I would place the attribution outside of the main section, and create a separate footer containing it. Then, you should remove "display: flex" and "align-items:center" from the body tag and place them in the .main tag. This way your footer will appear at the end at all screen-sizes and will be not affected by the flexbox.

    Also, you should remove .card {"margin-top: 18rem"} and .main {"height:100vh} from your media (this crops your content only on the height of mobile screen).

    Suggestions for accesibility:

    • instead of <section class="main>, use just <main> tag;

    Hope this helps!

    0
  • Hana•960
    @Hanka8Posted over 2 years ago

    Hi Osama, nice job!

    When you use "%" on width, the card will shrink on smaller screens. Instead, you can use "rem" - then the cards width will remain the same even on smaller screens, so you wont need any special media queries for responsiveness.

    Other suggestions:

    • use "rem" istead of "px" for "font-size", so the users can change it in their browsers - its considered a better habit for accesibility;

    • its better to use semantic elements instead of non-semantic, so you should use <main> instead of <div>;

    • headings should start at <h1> level, also the document should contain at least one level-one heading, so you should use <h1> instead of <h2>;

    Keep up the good work! ☺

    Marked as helpful
    1
  • Hana•960
    @Hanka8Posted over 2 years ago

    Hi Jimmy, nice job!

    My suggestions:

    • You can add multiple classes in "element.classList.add()" or "element.classList.remove()", so your JS code will be a little bit shorter;

    • when there is no text content in the <button> tag, you should add the "aria-label" to label the element or "aria-labelledby" referencing to some other element with accesible name;

    • there are no margins around the content in the design;

    Other than that your solution is great, keep up the good work!

    0
  • Hana•960
    @Hanka8Posted over 2 years ago

    Hi Tomasso, nice job!

    The main difference between ids and classes is that id can be used only on one element, while classes can be use for one ore more elements. The second thing is that ids are more specific, so they take precedence in your css. Generally classes are more common, because you can reuse them on other elements in greater projects.

    Other suggestions:

    • you should prefer semantic elements against non-semantic, so use <main> instead of <div>;

    • the document should have at least one <h1> landmark, so use <h1> on the heading instead of <p>;

    • the shadow around the card appears to be a little bit lighter than your solution, also the heading has a greater "font-weight"

    Keep up the good work!

    0
  • Maca•630
    @Maacaa0Submitted over 2 years ago

    I was not sure how to position the word Change in the Anual plan box. I had to set position: relative and set coordinates :D ... does not feel right.

    Also Iam not sure if i can set different background for mobile devices, since there is (images folder) another background image for mobile.

    Any suggestion is welcome. Thank you.

    order-summary-component HTML CSS

    1
    Hana•960
    @Hanka8Posted over 2 years ago

    Hi Maca, nice work!

    here are some suggestions:

    • you can use "min-height: 100vh" on body, so it takes 100% of the viewport, then apply "display: flex", "justify-content: center" and "align-items: center" to have your card centered nicely;

    • if you want to center something horizontally, you can use "position: relative", "top: 50%", "transform: translateY(-50%) like this https://codepen.io/hanka-marukeviov-/pen/XWZJmoM

    • you should add some "@media" and change the size of the card on mobile screen

    Happy coding! ☺☺☺

    Marked as helpful
    1
  • Adriel•20
    @izucaSubmitted over 2 years ago

    I found it very difficult to make a responsive Card, any suggestions?

    Any suggestion on how can I improve my coding in general?

    NFT Card Solution

    1
    Hana•960
    @Hanka8Posted over 2 years ago

    Hi, you should define the width of the container, so every of its child (images, text etc) sits in it.

    I dont understand why you have such a big container margins - if you want to center it, use " display: block, margin-left: auto, margin-right: auto"

    Also you have some unnesecary divs, you dont have to create a div for every element. Just give them a class instead.

    Hope this helps!

    0