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
OverviewSolutions10Comments77

All comments

  • Marit• 400

    @Maritxx

    Submitted

    Hi! this is my solution for the tip calculator project. I struggled with the custom button (updating the value for that) and the updating of values when the input changes. Is there a smart way of doing this?

    Tip calculator using javascript

    1

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey, great work on the challenge! You can use input eventListener to achieve that. For custom input it would be:

    document.getElementById("custom__button").addEventListener('input', (e) => {
	calculateTipAmount(e, e.target.value / 100)
})

    You can apply the same to other inputs. Happy coding :)

    0
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey Catherine, excellent work on the challenge! There is just a little gotcha about background-size. Header has fixed height so contain won't let the image grow as the screen gets wider. Everything else looks perfect, happy coding :)

    0
  • P
    Darya• 160

    @DHolets99

    Submitted

    Finally finished this challenge. There were difficulties with the positioning of the main image. Feedback is welcome

    Responsive landing page

    1

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey Darya, design is almost pixel perfect and responsive so nothing much to add, excellent work! Some suggestions:

    • Between 450-720px text and image overlaps. You can change the breakpoint for mobile to 720px. My general suggestion in this regard would be to take mobile first approach so you wouldn't be dealing overflows. Also desktop layouts are more complex than mobile. So you can achieve the same result with less code since mobile usually just 1 column layout. Although you'd still need to check desktop before styling for mobile to make the transition easier from simple layout to complex.
    • Sections aren't landmark elements by default. If you wish to use as one, you need aria-label or title attributes. Or you can simply use it inside <main>.
    • You can reduce the usage of <div>. If there is a semantic element it's always better to use one instead of div. In your case you can use <p> for texts. Also you can get rid of nested divs like in here:
    <header>
    <div class="wrapper">
      <div class="header">
        /* content */
      </div>
    </div>
  </header>

    All 3 containers are parent of the content and you can achieve the same by just using <header> without the need of divs. In regards of accessibility nested divs won't mean much because they're ignored, just there for styling purposes. Although not using unnecessary elements will keep your code more maintainable and easier to work on for others. Aside these everything looks great and happy coding :)

    0
  • Anderson Vianna• 240

    @andykallian

    Submitted

    Hello!

    It's my first time trying to implement a dark mode theme, and it was more difficult than i've thought.

    First of all, creating an empty div and then populate it with classes and events through JS code required a lot of attention as they were "a div inside another div inside another div...and so on"

    after, I couldn't turn 'checkbox' input into rounded instead a square box, so I created a div (through JS) and then put a style on it in css. Visually speaking, it worked! Functionally speaking? well...I can't confirm this since I still haven't added the functionality to confirm the check by the user.

    Anyway, my logic in JS seems to be working, but I don't believe it's following good patterns.

    if anyone can give any suggestions related to the whole project, it will be welcome and I will be grateful!

    I hope you enjoy it! Regards!

    P.S: I still intend implement media queries, but for now it's ok

    To-do List

    2

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey Anderson. Great work on your solution! To use checkbox and style as you wish you can use appearance: none. This is what I've used on my solution:

    .theme-switch {
    appearance: none;
    width: 26px;
    height: 26px;
    background-image: url('images/icon-sun.svg');
    background-repeat: no-repeat;
    cursor: pointer;
}

.theme-switch:checked {
    background-image: url('images/icon-moon.svg');
    background-position: center;
}

    Also you can define just one class per theme on body. Example:

    .dark {
    --bg-color: hsl(235, 21%, 11%);
    --bg-content: hsl(235, 24%, 19%);
}

.light {
    --bg-color: rgb(235, 235, 235);
    --bg-content: white;
}

    Then add & remove with checkbox. You can use the variables on any element, switching class from body will be enough. Would make it much more easier to store in localStorage as well. Happy coding :)

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, congrats on your solution! Design looks good, responsive and you have clean usage of CSS. Nothing much to add but some suggestions:

    • If you wish to place the attribution to bottom of page without effecting your container's placement you can use:
    .attribution {
  ...
  position: absolute;
  bottom: 0;
}

body {
  ...
  justify-content: center;
}
/* In this case you can place your container to center */
    • Background color is missing on body: background-color: var(--Very-light-gray); Although arguably white looks better so you might as well leave it as it is.
    • Instead using <div> on .container and .attribution you can use semantic elements to make the page accessible: <main class="container"> & <footer class="attribution">
    • You shouldn't leave alt empty. Screen readers skips empty alt. Since images on this challenge are for decorative usage, nothing wrong regarding accessibility. But your page might get lower SEO. Instead you can use like : <img src="./images/icon-sedans.svg" alt="sedan" aria-hidden="true"/>
    • On production, when you dirct links with target="_blank", adding rel="noopener noreferrer" will make it secure to attacks. Aside these nothing I'd add, happy coding :)

    Marked as helpful

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, excellent work with the challenge! Design looks good and accessible. Your scss usage looks alright and for such project you won't need more of it's features. By the time projects starts getting bigger you can start structuring your scss files like _variables, _mixins and import to main. But would be redundant for this one. Some suggestions.

    • Right now your card isn't responsive and that is due to the fixed width: 375px value on container. If you use max-width you'll see it's responsive for smaller screens aswell. By default width: auto which is taking available space to fit the content inside it. In this case you can increase the value to match the desktop design by keeping it same for 375 screen width: max-width: 450px;
    • There is a vertical scrollbar even though nothing overflows. That's due to the default margin on body. If you give margin: 0 to body it'll fix that. For your next projects I'd suggest resetting all padding and margin at the start of your project by using * { ... } selector. So you'd have full control over your elements. Aside these nothing I'd add, happy coding :)

    Marked as helpful

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, great work on your solution! To place attribution on bottom and not effect the rest of your layout you can use:

    .attribution { 
    font-size: 11px; 
    position: absolute;
    bottom: 0;
}

    Absolute positions the element relative to it's parent without effecting any other element. Since parent of attribution is the main, you'd want to move it out of main and make it's parent body. To keep it semantic, you can use <footer> on attribution instead of div. Hope this answers your questions, happy coding :)

    0
  • Mihajlo Rangelov• 70

    @nekokanbaru

    Submitted

    I tried to input the close button svg using svgsprite to change the color on hover, but when I did it the element was there but you couldn't see the icon, so I changed it back to a regular img.

    Another problem I had is that I could load tasks from local JSON but not save them, I looked it up on google but everywhere I looked people were using node, so if someone knows any other way I could do that I would be grateful if someone could tell me.

    Otherwise it was a very fun project.

    Todo app (HTML, CSS, JS)

    1

    Eray• 1,430

    @ErayBarslan

    Posted

    Hello Sylvanas! Great work on the challenge. It's responsive and everything works fine.

    • You can use visibility:hidden by default and turn it to visible on hover to achieve that effect.
    • To save data if you're not down to use a database which would be too much for this challenge, your option is localStorage. Whenever user creates a todo you can use localStorage.setItem() method to save the data and call with localStorage.getItem() method. You'd need some checking but it's pretty straightforward. If you wish to see on usage you can check my solution of the challenge which I used localStorage: Todo App. Hope these answers your questions, happy coding :)

    Marked as helpful

    1
  • Aman• 280

    @AmanGupta1703

    Submitted

    .container::before { content: ""; background: url("/images/pattern-background-desktop.svg") center center/contain; position: absolute; width: 100%; height: 26rem; top: 0; left: 0; z-index: -1; } I am able to see the output of the above code in the visual studio live server, but I can't see it in the GitHub live page.

    I would like to hear your feedback! 😊

    Order-Summary-Component

    2

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, nice work with your solution! Some suggestions:

    • Regarding html structure, you have 3 nested <div> containers which you can achieve the same result by just using one. You can simply remove .summary-box without effecting the design. Removing .container will require little refactoring but you can remove anything related to that.
    • Using background on container breaks it's design on screen change. Using on body would be better option. After removing these elements from html you can use:
    body {
  ...
  background: url("/images/pattern-background-desktop.svg");
  background-repeat: repeat-x;
}

.summary-item {
  ...
  width: 90%;
  max-width: 414px;
}

    As you can see, just adding max-width is enough to achieve the same thing. You can also lower the need of media-query by adjusting % values for the mobile version, then give it a max-width. This approach is also preventing content overflowing between 375-550px due to width: 48%.

    • Lastly, now we have just .summary-item as container, you can use <main> on it instead div to be semantically right. Aside these good work again and happy coding :)

    Marked as helpful

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, excellent work on your solution! Design looks good and everything works as supposed to. My suggestions:

    • Functions fire on input change with key, however nothing happens when numbers changed by spin box. You can listen to input change through javascript. There is oninput listener on html but doesn't work same way:
    bill.addEventListener("input", () => { billGen(); renderBill() })
customTip.addEventListener("input", () => { customTipGenerator(); renderBill() })
people.addEventListener("input", () => { peopleCountGenerator(); renderBill() })

/* Since there are two functions for each listener, we put them inside another function. */
    • Alternatively if you wish to not deal with spin box you can simply use text which is the general convention and you're already converting input by Number(x). Also on modern browsers to remove spin box from number input you can add to css:
    input[type="number"]::-webkit-outer-spin-button,
input[type="number"]::-webkit-inner-spin-button {
    -webkit-appearance: none;
}
input[type="number"] {
    -moz-appearance: textfield;
}
    • Instead of <h2> It'd be better use <h1> for the heading of page. If there are several heading then It'd be applicable to use other headings.
    • For .attribution you can use <footer> instead <div>. Aside these excellent work again and happy coding :)

    Marked as helpful

    0
  • Luis Olivero• 170

    @Luis-Olivero

    Submitted

    How do I get rid of the space between the profile picture and the profile name/age?

    How do I get rid of the space between the stats and the text under the stats?

    Thanks for all the feed back!

    profile-card-component-main

    2

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey Luis. Great work on your solution! It's responsive and accessible. I've looked into your code and the issue comes from top attribute on img. You can fix it by changing it to margin-top :

    .card__img--picture img {
    position: relative;
    margin-top: -3rem;
    border: 6px solid #fff;
}

    When top attribute used, base position of the element stays same, just visually effects it's positioning. Using margin simply fixes that. For your second issue, it's due to the default margin of h2 and p. You can fix by using: h2, p {margin: 0;} To not deal with default margin, padding at the beginning of CSS in addition to box-sizing I'd suggest adding:

    *,
*::before,
*::after {
    box-sizing: border-box;
    margin: 0;
    padding: 0;
}

    Aside these perfect solution and happy coding :)

    Marked as helpful

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, excellent work on this one! Regarding your question, another option would be adding a return button. You can use setTimeout here but it displays only for 1,5 seconds which isn't enough for the user to read the page. It's better to use a higher value. Additional suggestions:

    • Right now user can submit without selecting a rate and it returns undefined value. To prevent this you can add an if state to your submit event listener function like:
    submit.addEventListener("click", () => {
  if (a) {
  /* your entire function */
}
})
    • On screens smaller than 400px, content overflows out of body. That is due to using fixed width value. Instead you can use max-width: 385px; to make your container responsive. By default it's width: auto which is taking the available space without overflowing. Giving fixed width overrides this. Aside these nothing much I'd add and happy coding :)

    Marked as helpful

    0
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, great work on this one! Design looks good and is responsive. My suggestions will be regarding semantic html:

    • Using list on navigation elements (About, Careers, etc.) is right approach. Though you should wrap <ul> with <nav>...</nav> for better usage. Since there would be two <nav> in page. You can use label on them like aria-label="top navigation"
    • Again using list on socials is right appraoch. But they'll be links directing to the social page in production. So you should wrap your <li> with <a> like : <a href="#"><li>...</li></a>
    • For copyright instead <p>, <small> would be a better usage. Also when you use target="_blank" attribute on links, adding rel="noopener noreferrer" will make it secure. Aside these everything looks great. Hope you find these helpful, happy coding :)

    Marked as helpful

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, congrats on your first solution! Design looks really close to the provided. My suggestions:

    • Instead of fixed width you can use max-width to make the page responsive. by default width: auto which is responsive. As it is, on small screens content overflows.
    • Unless you really need to, don't give height to your elements. Let the width,margin,padding define their height so you won't run into any issues. I've done some refactor to make the page responsive:
        .wrapQR {
      background-color: hsl(0, 0%, 100%);
      max-width: 300px;
      padding: 0 10px 30px 10px;
      border: solid #fff;
      border-radius: 15px;
      margin: auto;
    }
    .QR {
      width: 100%;
      border-radius: 15px;
      margin: 10px auto;
    }

    You can keep the rest of styling as it is. With these changes, base design is the same but when screen gets smaller there is no overflowing issue that is due to max-width. Also since your page is deployed now, you can use generate new screenshot option to see how close you are on your design. Hope these helps, happy coding :)

    Marked as helpful

    1
  • Eduardo• 200

    @EDDuardOo-Code

    Submitted

    I would like to ask how to improved with the management of spaces, using paddings or margins?

    also the use of media querys and until which point these are useful ?

    also, what are the best practices to work with css, flexbox and media querys?

    QR code component

    #accessibility

    2

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, congrats on your first solution! Some suggestions:

    • It's good to get use to media-query. But using it too much might have side effects on responsiveness, taking the smooth transition away. Also card becomes way too big on wider screens since it's always relative to body. For this challenge you can use max-width without the need of media-query.
    • You don't need container div. You can center your card relative to body. By removing all styling on media-query and .container you can use:
    body {
    min-height: 100vh;
    display: flex;
    justify-content: center;
    align-items: center;
}

.child{
    ...
    width: 95%;
    max-width: 300px;
}

.attribution { 
     ...
     position: absolute;
     bottom: 0;
}
/* position absolute on attribution perfectly centers the container */

    Test it out and you'll see the difference. Also as far as html goes, you can use semantic landmarks instead using div on everything. By removing the container your html inside body can look like:

      <main class="child">
    <img src="./images/image-qr-code.png" alt="image-qr-code">
    <br>
    <div class="text">
      <h1 class="mob"> Improve your front-end <br> skills by building projects </h1>
      <br>
      <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
    </div>
    <br>
  </main>

  <footer class="attribution">
    Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank" rel="noopener noreferrer">Frontend Mentor</a>.
/* when you link to other pages, adding rel attribute adds security */
    Coded by <a href="#">Your Name Here</a>.
  </footer>

    I haven't touch <br> but for spacing it's better to use margin through css instead br. Hopefully these helps, happy coding :)

    Marked as helpful

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, excellent work with your solution! Design looks good, it's responsive and js sidebar functionality is clean. Some suggestions:

    • There is a little gotcha on sidebar. Close and open buttons works as supposed to, but while it's open if the screen switches to desktop layout it would be better to close it by default which you can achieve by just adding .show-sidebar {display: none;} to your media query.
    • When the screen gets wider than 1440px container keeps growing since width is auto and content aligns to right side of screen breaking the design. You can use max-width and add auto to margin to center the container:
      .container {
    max-width: 100rem;
    margin: 5rem auto;
  }

    You can do the same to the header if you like. Aside these nothing I'd add. Excellent work again and happy coding :)

    Marked as helpful

    0
  • Wellinton Tews• 10

    @wellintontews

    Submitted

    Project made with HTML and CSS, I'm looking for ideas to develop myself more and more in this magnificent world that is the front-end, all kinds of help and suggestions will be welcome

    Project made with HTML and CSS

    1

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, nice work on your solution! Some suggestions:

    • You're using media-query on screens smaller than 375px for mobile version, although content overflows until 700px. For safer approach I suggest designing mobile version first then style for bigger screens by using min-width so you won't ever deal with overflowing issues. Also desktop layouts are usually more complex. By designing desktop first then switch mobile, we're taking that complexity away. So with mobile first approach, we'd be writing less code.
    • Unless you really need to, I'd suggest not giving height to your containers and always let it's children, padding, margin define it's height. Won't matter much for a static card but once you start working with dynamic content, you'd end up with lots of overflowing.
    • Lastly, instead of using fixed width on your containers you can use max-width to keep your containers responsive. Because by default width: auto which is responsive and giving fix value overrides it. Hope these suggestions helps you, happy coding :)
    0
  • Hayamaker• 10

    @Hayamaker

    Submitted

    First time building a project in Frontend Mentor!! I learned a lot about how margins and paddings worked, how shadow boxes were implemented, how websites were deployed and many more. Quite excited to build more projects from here!

    QR Code Reader by hayam4k3r

    #foundation#materialize-css

    1

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, amazing work as your first challenge! Instead of margin you can center you card by the help of flex:

    html {
  height: 100%;
}

body {
  min-height: 100%;
  display: flex;
  justify-content: center;
  align-items: center;
}

    Also I see you're trying to deploy your site using github-pages. Simply change your html file name to index.html because pages looks for an index to run the site. Try it out then your site should be deployed with no issues. Hope these helps and happy coding :)

    Marked as helpful

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, excellent work with your solution! Design looks good, it's responsive and form works as supposed to. There are just few things I'd add to your solution:

    • By the time screen gets wider, content keeps stretching out and text eventually becomes one line and makes it harder to read. You'd want to use max-width on container elements so that design won't break.
    • When you use sectioning elements like <section>, <article>. If there is a fitting heading It'd be better use one for accessibility. In this case you can use <h2> for Try it for free text instead of <p>.
    • Lastly on mobile version, there is extra padding and margin adding 40rem to top of screen. Your form is already centered so you won't need it. Aside these everything looks great and happy coding :)

    Marked as helpful

    0
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, github pages looks for index.html on root directory. In your case all your files are inside qr-code-component-main and github pages detects no index.html. If you move all the files inside it and simply remove the file, it should deploy with no issues.

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, nice work with this one! I see you've tried to center your content but you need an addition for it to work:

    html {
    height: 100%;
}

body {
    ...
    min-height: 100%;
}
    • You should move background-color and image from .body-wrapper to body element. Right now background doesn't cover wide screens. Use the wrapper just to align your content.
    • Also if you use max-width on containers instead of a fixed width, your page becomes more responsive.
    • You have a good usage of media-query, but you might want to increase min-width value as content overflows on certain screens. These are my suggestions to improve your solution, happy coding :)

    Marked as helpful

    1
  • Stacey• 30

    @staceysav

    Submitted

    Feedback will be highly appreciated! While doing the project I had some difficulties; if you have answers to any of the questions below, please let me know :)

    • How to add waves to the background?
    • The shadow doesn't look as good as in the layout. Is there a change to do the same?
    • I tried to add margins to the "Order summary" line and the paragraph below it, but they didn't seem to work. Where to look to find the root of the problem?

    Order summary component using Bootstrap

    2

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, you can add background to body as:

    body {
    ...
    background-image: url(./images/pattern-background-desktop.svg);
    background-repeat: repeat-x;
    /* we can use repeat x for this specific image so that it supports wider screens*/
}

    Try out box-shadow: 0 25px 20px hsl(225, 67%, 88%); which is what I've used on my solution.

    -Instead of <h4> you should be using <h1> for Order summary, each page requires a level one heading. You can style with css as you like. Also their level should change 1 by 1 so you can change <h6> to <h1>.

    • alt should have meaningful explanation. Instead of alt="..." you can use alt="dancing with music illustration" These are my suggestions to improve the solution. Happy coding :)

    Marked as helpful

    1
  • Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, great work on your solution congrats! Some suggestions:

    • You can use mix-blend-mode without the need of ::after and you'd get much closer to the design. If mix-blend-mode wouldn't exist that's a nice solution though! By removing after you can use:
    .right img {
 width: 100%;
 display: block;
 opacity: .7;
 mix-blend-mode: multiply;
}

.right {
  background-color:hsl(277, 64%, 61%) ;
}
    • You have a media query for small screens but your page doesn't support between 468-1000px and content overflows. I'd suggest designing mobile version first then use min-width to design for bigger screens. This way you wouldn't deal with overflow and in general convenient approach is mobile first design. Also It'd be better to use max-width on containers instead width. By default width:auto and is responsive, overriding it takes the responsiveness away. Aside these great work and happy coding :)
    0
  • Queen• 60

    @UmesiQueen

    Submitted

    Hey Guys, I'm really having fun completing these challenges . I found this one interesting trying to figure out how to overlay a background color on the image and Yes! it was the fastest I've completed so far 🤣

    Your Feedback will be appreciated.

    Stats preview card component

    1

    Eray• 1,430

    @ErayBarslan

    Posted

    Hey there, excellent work on this one and good to hear you're having fun :) Some suggestions:

    • To overlay, on .background_imgyou can use mix-blend-mode: multiply; so you'd match with design.
    • Content overflows between 375-900px. I'd suggest designing mobile version first and instead of a fixed width on main you can use max-width so that your page would be responsive for all sizes with the same amount of code. By default width:auto which fills the available space. By overriding it we take the responsiveness away.
    • For semantic markup you should use landmark elements to wrap your elements instead of div like : <main class="container"> & <footer class="attribution">. Your designs stays the same but page becomes more accesible. Happy coding :)

    Marked as helpful

    1