Skip to content
OverviewSolutions
19
Comments
15
Furk
@doganfurkan

All comments

  • Furk•600
    @doganfurkanPosted about 1 year ago

    Hey, good job! My suggestions would be:

    • Use aspect-ratio for a perfect round shape
    • Try to avoid fixed widths and heights as much as you can. Especially for mobile, where the screen is already limited.
    • Use em and rem units instead of px
    • Your flex-item-1 classed div has a negative margin-bottom for what? Remove those margins and use "gap" for better control over margins.

    I think if you handle those, you're going to end up with an excellent job.

    Marked as helpful
    1
  • Furk•600
    @doganfurkanPosted over 1 year ago

    Hey, congrats! This is a very good job. I want to help you with one thing, though: Your "Bookmark" button. Your icon there isn't aligned with the button.

    • For the bookmark button; remove the padding, and set justify-content to "space-between".
    • For ".divIcon", get rid of the absolute positioning.
    • For the icon image; set the margin to 0 and "vertical-align" to "bottom".
    • Set "align-items: center" for Btnbookmark div because it messes up the height of your bookmark button
    • And lastly, for bookmarkText div; set flex-grow to 1 and justify-content to center.

    I think these will solve the issue. And again, good job!

    Marked as helpful
    0
  • Furk•600
    @doganfurkanPosted over 1 year ago

    Good work, but still need some fixes as anything isn't perfect:

    • I think we can start with an advice, instead of a blank card
    • Advice number is meant to be the advice id that is coming from the API, it is not that wrong with your approach but in the design it was id
    • Your fetch function has "cache". This is why it sometimes doesn't reconnect to the API. If you have a look at here you can learn more about it

    Congrats

    Marked as helpful
    0
  • Musstah•40
    @MusstahSubmitted over 1 year ago

    Hi all!

    This is my first attempt to the first challenge on "frontendmentor.io". I have just started learning WEB development, any feedback will be appreciated.

    Product card with CSS and Flex

    2
    Furk•600
    @doganfurkanPosted over 1 year ago

    Wow, have you really "just" started learning web development? This is pretty darn good for a starter. Just a couple of things here:

    • You see, in the design the word "Perfume" is written in uppercase. For places like that, where you want to show text in uppercase no matter the HTML, you can use text-transform: uppercase in your CSS.
    • Your card has a width of 600px, and it doesn't shrink, at all. Because of that, I think you should set your breakpoint for the mobile view as 600px in your media query. Right now, someone with a screen width of 500px would suffer to view the whole card. You can just switch to mobile view under 600px.
    • I don't know if you noticed, but you have a little gap under the image in mobile view. That is the default behavior of the <img/> tag. You can add vertical-align: bottom to your CSS to get rid of that irritating gap.

    Apart from all that, it is mind-boggling that you just achieved this as a beginner. Huge Respect!

    Marked as helpful
    0
  • Earl Villapaz•80
    @iEarlGSubmitted over 1 year ago

    Hello everyone! 👋

    I'm out for a week because of a mentorship 😅 I learned a lot but at the same time I can't get over about this challenge. I finished the 3-column preview card component! It felt amazing to complete my 1st challenge this month <3.

    What did you find difficult while building the project?

    • The responsiveness of the website took me too long to process, but I think I nailed it, thanks to the feedback on my last challenges😅. I make the mobile side first but I got lost and the height & width on the desktop is not the same from the challenge it seems too small I guess? any feedback to fix this I would really appreciate it <3

    Which areas of your code are you unsure of?

    • I'm not sure about the the height & width on the desktop side is not the same from the challenge it seems too small I guess?

    Do you have any questions about best practices?

    • Please feel free to input any best practices that you find not best on my code I would really appreciate any feedbacks and best tips from you guys!!

    Happy weekend coding, guys!! 👨‍💻💙

    3-column preview card component

    1
    Furk•600
    @doganfurkanPosted over 1 year ago

    Hello! Good job, congrats! Here are some quick fixes that you can do:

    • Your breakpoint for your media query is 1440px. It is a lot. I think you should bring it down to somewhere like 700px.
    • Your main problem with the width and height is due to your not assigning a max-width to your container. If you assign .container{max-width:1000px} you can restrict the maximum width of the design.
    • For mobile view, you are using align-items: center for your container. This results in each container having different widths. It is not the most pleasing thing to see. Just get rid of that and let them have 100% width of the container.
    • At last, your buttons can use some more margin-top for the desktop view.

    Again, good job! Keep coding!

    Marked as helpful
    0
  • Bosca Darius•120
    @dariuss1123Submitted over 1 year ago

    This is my take for this challenge. I've used HTML, CSS and JavScript. I used random numbers for the values but i didn't managed to link the button. Any suggestions for this or anything else are welcome. Also i used media query for the mobile version using a mobile-first approach.

    Results summary component (my take) HTML, CSS, JS

    5
    Furk•600
    @doganfurkanPosted over 1 year ago

    Hello there! You did a good job, congrats! I've some suggestions though:

    • Since you gave your body a height of 100vh, it sometimes doesn't show the whole content. If you change it to min-height, at least, you would give it room to scroll through content.
    • You have a div with the class of result-div, and you are using percentages to give it a size. Don't declare both the height and width. To have it as a perfect circle, I recommend giving it width: 50% (for example); aspect-ratio:1 styling. Aspect-ratio makes the height and width equal.
    • Your result-div doesn't show the elements in the center. If you make it a flexbox, assign the flex-direction as column, and apply justify-content: center you can have them centered. Also, get rid of that <br/> tag to adjust the gap.
    • Why do you limit your container to 375px for mobile? Your breakpoint is somewhat like 400px already. I think, do not leave that little space on the sides. Give it a width:100% for mobile and increase the padding.

    I think just those can make a big difference in appearance.

    Have fun building!

    Marked as helpful
    1
  • Furk•600
    @doganfurkanPosted over 1 year ago

    I advise you to have a look at:

    display: grid;
grid-template: "here-comes-this here-comes-that"; // that's just a random example
grid-area: "here-comes-this";

    These kinds of designs are better done with grid rather than flex. I believe that once you do it with grid, your design will be a lot more similar.

    0
  • Furk•600
    @doganfurkanPosted over 1 year ago

    One quick fix would be giving max-width to the main content div instead of the left content div. You see, you have a huge gap between the "The Bright Future of Web 3.0" and "New" sections. If you change the element that has max-width, it should fix that.

    Also, it doesn't appear like your solution has the features to be tagged as "#jss #preact #react-query #solid-js #material-ui". I would recommend using true tags. It's not changing anything, but that would be more accurate.

    1
  • Furkan Y.•80
    @frkanyilmaz2Submitted over 1 year ago

    Hello there, I've tried to make this responsive with react. When I lower the screen height my top element goes beyond the border and I can't scroll to see what's there. How can I solve this?

    In summary section I mapped through data.json to list the results. But in order to change both text color and background color I put two more properties inside data.json as color and bg-color. And in <li> I put them inside style attribute. My second question is; What kind of logic should I implement to use right colors without altering data.json?

    Results Summary Component with React

    1
    Furk•600
    @doganfurkanPosted over 1 year ago

    Adaşım merhaba, işimi kolaylaştırdığı için direkt Türkçe yazacağım. (If you don't know Turkish let me know). Öncelikle mobil görünüm için container'ının position: absolute'a, kesin height ve width'e, transform'a ihtiyacı yok. width:100% height:auto; işini görecektir. Bu düzenleme üst kısımda görülemeyen "Your Result" yazısını ekranın içine almaya yetmeli.

    Renkler konusunda da React'tan faydalanbilirsin. Eğer summary'deki her bir li'yi (ki ben olsam div kullanırdım) bir component haline getirip json'dan aldığın veri sayısında componenti yazdırırsan ve de component'e rengi props olarak geçersen arkaplanda aynı rengin saydamlaştırılmış halini, yazıda da rengin kendisini kullanabilirsin.

    Bunların dışında birkaç renk ve boyut hataların bulunuyor.

    Umarım karışık olmamıştır. Yaptığın iş için tebrik ederim. Eğer yorumum sana yardımcı olduysa "Mark as useful" dersen memnun kalırım. Kolay gelsin!

    1
  • Furk•600
    @doganfurkanPosted over 1 year ago

    I suggest you have a look at flex boxes. You shouldn't be using that much absolute positioning. My page structure would be like this:

    <body>
  <main id="card">
     <section id="result">{Here is where the "Your Result" section going to be}</section>
     <section id="summary">{Here is where "Summary" section going to be}</section>
  </main>
</body>

    And then, I would use flex boxes to position them:

    // This container styling positions my card at the center
body{
  width:100%;
  height:100%;
  display: flex;
  justify-content: center;
  align-items: center;
}

// This card styling makes my Result and Summary section side to side without using absolute positioning
#card{
  display: flex;
  background:#fff;
  box-shadow:3rem 3rem 5rem rgba(0,0,0,0.25);
  border-radius: 1rem;
}

    This is what I'm suggesting to you to kick off your flexbox knowledge. If you want to learn more about flexbox, you can click here But if you want to see how my structure would work in real life, I also completed this challenge, and you can have a look at it by clicking here

    You are off to a good start, I'm sure you can develop your knowledge by doing projects like this. If you have any further questions, I will be happy to help.

    At last, I would appreciate it if you find and mark my comment as useful.

    Marked as helpful
    1
  • Aharen-Pranata•30
    @AlvinSetyaPranataSubmitted over 1 year ago

    The problems that i encounter are:

    • The gap inside of the container is bigger from the design. i've already not to use gap but still bigger

    how can i shrink the gap space inside of my container that does look like in design?

    Starter Project

    2
    Furk•600
    @doganfurkanPosted over 1 year ago

    Your h2 and p elements have their default margins. You can see default values here: w3schools They are the reason there is a gap there. You can give them a margin of 0.

    A little tip for me is to start styling with cleaning default alignments. You can use *{ margin:0; padding:0; box-sizing: border-box; } to get rid of default alignments

    1
  • Furk•600
    @doganfurkanPosted over 1 year ago

    Firstly congrats, I think I've got some feedbacks for you:

    • Your "Learn more" button has a border of "1.5px" on hover. You shouldn't be splitting pixels into 2. Dimensions in pixels should be integers.
    • Again your "Learn more" button, could use a border with the color of the background, so that it wouldn't mess up the placement of the footer. What I'm trying to say is add a black border at start, so you wouldn't make it bigger on hover.
    • Your menu comes open on mobile. We usually don't see it like that in real life, do we?
    • You are giving display: none to the menu when closed. What about hiding it on the right, out of screen, rather than deleting it from the screen?
    • When i used inspect to have a look at your mobile version, i closed the menu and when i turned back to the desktop version menu was not there. It is related with your display none decision.
    1