Skip to content
OverviewSolutions
8
Comments
13
Joanna
@JoannaLapa

All comments

  • Lauryne•260
    @lauryne921Submitted about 2 years ago

    Tell me what you think if the responsive is good or not. I liked doing this project so if something is wrong tell me

    Responsive Testimonials Grid

    2
    Joanna•370
    @JoannaLapaPosted about 2 years ago

    Hi LAURYNE’S,

    congratulations on completing the project! This is the best way to make the progress.

    Concerning your question - I suggest you to correct the mobile layout. Try to use Mobile First approach which is a good practice. Your solution with @media screen and (max-width: 375px) cause that all screens over 375 px (so even 376px) have desktop layout and everything is very very small. My suggestion is to prepare at first styles for mobile layout and after change the layout for bigger sizes with eg. @media screen and (max-width: 560px) etc. If you're not familiar with Mobile First approach you can read more about here.

    Other suggestions to improve your code:

    1. You don't need to repeat yourself with background-color, color and other design styles which are the same for all sizes - you can delete them from @media, because you styled them already in general code.

    2. To divide your articles (.one, .two, .three, .four) you can use gap on parent (.wrapper in your project) instead of margin (much less code). Gap works for grid and flex - you can read more here on mdn

    3. Instead of code: grid-column-start: 2; grid-column-end: 4; grid-row-start: 2; grid-row-end: 3; you can simply write a shorthand: grid-area: 2 / 2 / 3 / 4 more about grid-area here

    4. I feel that making position fixed on body and position absolute on global-content is not necessary.

    5. I can recommend you a video from Kevin Povel where he made exactly this project explaining by the way many grid cases, I think that it can be useful for you. And by the way I recommend you Kevin Powel's youtube canal, there is a lot of useful css tricks. Here is the video with this project.

    I hope that my comment was helpful! Happy coding ;) Joanna

    Marked as helpful
    1
  • Youssef Magdy•160
    @youssefmagdy21Submitted about 2 years ago

    The code is updated to fix a few bugs, shout out for @JoannaLapa for pointing them out ⭐

    • Fixed a bug where the advices weren't generated properly on Firefox.
    • Fixed a bug where the page didn't display properly on smaller screen sizes

    feature update 4/10/2022

    • Updated the button to be disabled for 2 seconds after generating an advice <the api website says it can generate a new advice every 2 seconds> so if the user wants a new advice he may wait 2 seconds noticing the button is disabled better than just clicking on the button really fast not knowing why it wouldn't get a new advice instantly...

    Responsive Advice Generator App

    1
    Joanna•370
    @JoannaLapaPosted about 2 years ago

    Hi Youssef Magdy!

    My congrats on finishing the challenge! Your solution looks really good - it is responsive and you took care of all the design details included in the design file. The application works well besides the Firefox browser (I explain below). Your code is clean and short.

    Some small details that you can improve: JS:

    1. When you open your site by Firefox browser there is a bug with taking advice from API - it takes only the advice at first and after doesn't refresh it. The problem doesn't exist on other browsers. Apparently, Firefox by default looks for a matching request in its HTTP cache. If there is a match and it is fresh, it will be returned from the cache - so it doesn't change the advice for a new one. You can read more in this article on mdn.

    You can resolve it by adding {cache: no-cache} just after the URL address. I think this is a very interesting topic.

    CSS:

    1. Very little detail to consider - you repeat 3 times the code: display: flex; align-items: center; justify-content: center; I think that this is a good way to add a class dedicated for flex like eg. "flex-center" and add it to HTML together with other classes.
    2. The responsiveness of your project is really good. The only suggestion that I have is when I look at iPhone 5 size there is no margin between the container and the edges of the screen.

    I hope that my feedback was helpful, Happy coding! Joanna

    Marked as helpful
    1
  • SAUBHAGYA ASHISH•30
    @saubhagyaashishSubmitted about 2 years ago

    Hi Viewers,

    Please check my solution and point out some of my mistakes and areas I need to Improve. The issue I know is that I have used the same CSS properties in multiple classes. Please give me some suggestions on how to improve it, what I mean that how should I avoid writing the same CSS property again and again for different classes. For example, I have used this thing multiple times - { display: flex; justify-content: center; align-items: center; }

    Responsive landing page using CSS Flex

    1
    Joanna•370
    @JoannaLapaPosted about 2 years ago

    Hi SAUBHAGYA ASHISH!

    Congratulations on finishing the project! It was my first project on frontendmentor, and I know already that making challenges helped me a lot in improving frontend skills. At this moment after a couple of months that I finished it I know that I should correct a few things ;).

    Coming back to your project:

    The main functionality works well and active states work well too. The desktop and tablet layout is pretty good. You used an input type radio with a label which I think is the best choice for this project.

    What I recommend to improve:

    At first, answering your question - to not repeat yourself you can create one common class for the same properties eg. flex { display: flex; justify-content: center; align-items: center; } And add this class to the tags in HTML where you want to use it - you can add several classes to the one tag, eg. <div class="container flex">, <div class="feedback-card-star flex"> . Another solution you can add the same properties like below: .container, .feedback-card-star, .votingReview { display: flex; align-items: center; justify-content: space-around; text-align: center; } And the code which is different you add separately like you already did in your project.

    1. I think that you didn't prepare the mobile version layout for 375px width. The width of .feedback-card is bigger than 375px so automatically it is bigger than the mobile screen. To correct that you can change dimensions using media queries starting your styles from the mobile view (you can read about the mobile-first approach). You can also try clamp() functionmore about clamp here or try to work with max-width and min-height. For responsiveness, I highly recommend the video of Kevin Powell and his free course. And his whole youtube channel is full of helpful CSS stuff.

    2. I noticed that you keep some styles code in the HTML file as. I recommend you to move everything to your index.css file as this is a good practice to separate HTML code and the CSS code.

    3. Concerning js file - it is a good practice to use const and let instead of var (in most cases the best choice is const). Const and let are the features that came with ES6. You can read more about the difference between them 3 here.

    I hope that my comment was helpful to you. Happy coding! Joanna

    Marked as helpful
    1
  • Joanna•370
    @JoannaLapaPosted about 2 years ago

    Hi Anton!

    My congratulations on finishing the project! Personally, I had also good feelings about this challenge. I noticed that your project is responsive on all devices also tablet - the layout looks good. I see that you took care of such details as a border on pictures and box-shadow on each article.

    Some improvements which I can recommend to you are connected with semantic issues.

    I see that you use span for: <span class="customer-name">Daniel Clifford</span> <span class="customer-state">Verified Graduate</span>

    In my opinion this is better to use here the tag <h2> or <p> for the "customer-name" and <p> for the "customer-state". I personally used <h2> for the "customer-name" in my project, but I see that you used in same section <h1> lower so (If I am not wrong) using <h2> before <h1> would be recognized as a mistake.

    The reason that I recommend you to replace <span> is that span has no semantic meaning and we should use it only when nothing other is possible. It is perfectly explained on mdn documentation "The <span> HTML element is a generic inline container for phrasing content, which does not inherently represent anything. It can be used to group elements for styling purposes (using the class or id attributes), or because they share attribute values, such as lang. It should be used only when no other semantic element is appropriate. <span> is very much like a <div> element, but <div> is a block-level element whereas a <span> is an inline element."

    You can also consider replacing <div> tag with <article> tag for:

    <div class="testimonial review-1"> etc. <article> is a correct usage for an user-comment [here on mdn more about <article>](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article).

    I also recommend you the video of Kevin Powell who made this project and published the video focusing on grid conception, you can compare your version and maybe you'll learn something new: Kevin Powell's solution on GitHub Kevin Powell's solution with an explanation on youtube

    I hope that you will find my feedback helpful, Happy coding! Joanna

    0
  • Joanna•370
    @JoannaLapaSubmitted about 2 years ago

    I am pretty happy to finish this project.

    I am mostly interested in opinions if my code is enough clean and if I respect all accessibility issues?

    I am open to any suggestions ;).

    Manage landing page solution

    2
    Joanna•370
    @JoannaLapaPosted about 2 years ago

    @ErayBarslan thank you for Your feedback! I think that I corrected all issues according to your comments.

    I think that the problem with my intro image was connected with the fix grid-templates-row value because I don't see this issue now on the 1600px width screen and more (in the meantime I deleted this property).

    Concerning aria-describedby on my input, I put this for my error message:

    <div> <input class="footer__input" type="email" name="email" id="email" aria-describedBy="message" aria-label="email" placeholder="Updates in your inbox…"> <p class="footer__input-msgtext " id="message"></p> </div> the message appears only when the user set an invalid email and is connected with ariadescribedby with same id ('message'), is that correct usage?

    Thanks a lot again for your feedback :) Happy coding! Joanna

    1
  • Joanna•370
    @JoannaLapaPosted over 2 years ago

    Hi Kawaiiakuma!

    My congratulations on finishing the project. Making projects is the best way of learning!

    Concerning your questions - the perfect source for you is the video of Kevin Powell who did this project and published the video focusing on grid conception. He explains many grid issues. The second source which I highly recommend you to improve your responsiveness skills is a free course from Kevin:

    1. Kevin Powell's solution on GitHub here and his solution with an explanation on youtube here
    2. Kevin Powell's free course Conquering Responsive Layouts here

    Looking at your code:

    1. I noticed that you use often position absolute. Try to avoid it as often as possible - I used it in this project only once if I remember well for a quote image.
    2. I suggest you start with a mobile-first approach (start creating your design from mobile to desktop).

    I guarantee you that if you learn with the 2 sources above you will resolve many responsive problems and everything would be much more clear for you.

    I also recommend some naming convention in your HTML classes - I found BEM convention useful, but there are others too. More about BEM methodology here. A good practice is to name your classes in a way understandable for other people reading your code

    I hope that you will find my feedback helpful, Happy coding! Joanna

    Marked as helpful
    0
  • lsh•160
    @SH-Lee2Submitted over 2 years ago

    Hello everyone, finally! I finished the project. But I'm not sure if the way I did it is correct. In particular, it was too difficult to do responsive design. If there is something strange, please feel free to tell me!

    GitHub user search app(React, Styled-components)

    1
    Joanna•370
    @JoannaLapaPosted over 2 years ago

    Hi LSH,

    My congratulations on finishing the project! I can find users and I see all the needed information correctly. I can switch between dark and light mode, it works. Good job!

    Some details that you can improve:

    Concerning main functionality:

    1. Website, Twitter, and company information should all be linked to those resources. On the website there is a mistake - it tries to join https://gitgithub-user-search-app.netlify.app/ and the user's website address, then when I try to open the resource the page is not found. Twitter and company information aren't linked to their resources (and there isn't a cursor pointer).

    2. When I click on the button and the input value is empty there should be an error 'No results'

    3. When I try to find a user by his full name with spaces (like Joanna Łapa) I see an error - you can resolve this case if you split the input value using the following methods: .value.split(' ').join(''). If this is not clear you can see it here with my solution

    Some very small design details if you would like to improve:

    1. Your h1 has a typo (it should be devfinder instead of debfinder).

    2. Your search icon is smaller than the original version.

    3. I feel the impression that the font-family Mono Space doesn't work on the username (h1).

    4. A hover on the button doesn't work.

    I hope that my feedback was helpful! Happy coding! Joanna

    1
  • TsuneWeb•260
    @TSune-webSubmitted over 2 years ago

    Hi, community!😄

    Here's my solution for the challenge. I'm wondering if I can improve on two points:

    1. Grid Layout 🖼 　　- I was sort of new to CSS grid layout and had to figure out how to implement it while learning the concepts from MDN docs. So, I'm not sure if my implementation is a practical solution.

    2. Positioning Contents ✍️ 　　- For contents in each card, I selected individual blocks, like h1 and p of the testimonial on the right, to put them in position. I'd like to know if there are any better ways or if I can position all of them at once.

    Happy coding! ☕️

    Testimonials Grid Section

    1
    Joanna•370
    @JoannaLapaPosted over 2 years ago

    Hi, TsuneWeb’s!

    My congratulations on finishing the project. It is almost pixel perfect, you take care of all details like box-shadow. I noticed that your project is responsive on all devices also tablet. Your CSS is clean, and well organized with your comments, I really like it.

    Concerning your questions I recommend you the video of Kevin Powell who made this project and published the video focusing on grid conception, you can compare your version and maybe you'll learn something new:

    1. Kevin Powell's solution on GitHub
    2. Kevin Powell's solution with an explanation on youtube
    3. for grid-column and grid-row you can also use shorthand grid-area - more info you can find in this article on mdn.

    I also recommend you this box-shadow generator it was useful for me in this project.

    I hope that you will find my feedback helpful, Happy coding! Joanna

    Marked as helpful
    2
  • Joanna•370
    @JoannaLapaPosted over 2 years ago

    Hi Martin!

    My congrats for finishing the challange! Your solution looks pretty good!

    Your problem makes me many troubles too. Apparently Firefox by default looks for a matching request in its HTTP cache. If there is a match and it is fresh, it will be returned from the cache- so it takes the previous advice. You can read more in this article on mdn: https://developer.mozilla.org/en-US/docs/Web/API/Request/cache .

    You can resolve it by adding {cache: no-cache} just after the URL address. You can also look how I resolve it in my project here: https://github.com/JoannaLapa/advice-app-challenge/blob/main/src/js/main.js

    One suggestion concerning your JS file - it is a good practice to use const and let instead of var (in most cases the best choice is const). Const and let are the features that came with ES6. You can read more about the difference between them 3 here: https://www.freecodecamp.org/news/var-let-and-const-whats-the-difference/

    I hope that it helps you! Happy coding! Joanna

    Marked as helpful
    1
  • Joanna•370
    @JoannaLapaPosted over 2 years ago

    @k-stopczynska my congrats on finishing the challenge!

    I like your solution, it's pretty good, the code is clean, most of your design is similar to attached desktop and mobile files. Good job!

    A couple of details that you can improve:

    The logo:

    • on mobile version is very small - I suggest to make it bigger.
    • on desktop version with height less than 800px looks like the proportions between width and height are are not the same.

    A tablet dimension (like iPad):

    • hero-image size is the same from mobile to tablet version - I feel that you can improve the design here (personally on my project I set the background-position: center and background-size: cover maybe you will find those solutions useful).
    • I feel that the text area could be narrower

    An input .email-form: according to the active state design when I write the email address the font color should be changed on black. Also I suggest to set 'outline: none' than when we click on input we don't see the blue outline. You can achieve those effects by the code below: .email-form:focus { outline: none; color: black; }

    You can set a box-shadow on button to make it more similar to the given design.

    Concerning javascript functionality - one of my personal email address with the end '@g.pl' is recognized as wrong by your regex expression.

    I made also this challenge and if you will find the time to look at my version and write any feedback it would be very nice and helpful.

    Greetings from Poland and happy coding ;) Joanna

    Marked as helpful
    0
  • Mohit kalme•150
    @mohitkalmeSubmitted over 2 years ago

    The Advice Slip API does not give random advice in firefox but works in chrome. It's either firefox bug of API bug not sure. I have also tried a little work around for this problem, checkout my script.js file of this project in Github Repo, The commented out lines . Any suggestions are very welcome. Thanks!

    Responsive Advice generator app

    1
    Joanna•370
    @JoannaLapaPosted over 2 years ago

    Hi Mohit kalme!

    My congrats for finishing the challange!

    Your problem makes me many troubles too, but I resolved it. The browser by default looks for a matching request in its HTTP cache. If there is a match and it is fresh, it will be returned from the cache- so it takes the previous advice. You can read more in this article on mdn https://developer.mozilla.org/en-US/docs/Web/API/Request/cache You can resolve it by adding {cache: no-cache} just after the URL address like below: const res = await fetch(API_ENDPOINT, {cache: no-cache});

    I hope that it helps you!

    Marked as helpful
    1
  • Joanna•370
    @JoannaLapaPosted over 2 years ago

    Hi Sudarshan!

    My congrats on finished project.

    The desktop design looks well and everything works well.

    My recommendation is to correct the mobile design for 375px - your card dimansions are bigger than the screen.

    Good luck!

    Marked as helpful
    1