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

All comments

  • PTMeen 150

    @PTMeen

    Submitted

    I notched some times we get the same advice in a row. Is there a way to ensure we get different advice every time we make an API call?

    Any feedback is appreciated 👏

    Faris P 2,810

    @FarisPalayi

    Posted

    I think it might be because of browser caching (not completely sure). To disable it in Axios, disable it in the header like this

    axios.get('https://api.adviceslip.com/advice', { headers: { "Cache-Control": "no-cache" } })
    

    Have fun coding ✨

    Marked as helpful

    0
  • Faris P 2,810

    @FarisPalayi

    Posted

    Hey, nice job on this one 👍. Here are some of my suggestions:

    • set the image alt values to alt="" if images are only used for decorative purposes
    • I think the alt value divider is unnecessary in the divider img because it doesn't provide any useful information or extra context. i.e. since it's decorative.
    • try adding a :focus state style to the button so that people using keyboard to navigate can use the site easily
    • set an aria-label attribute to the button so that the purpose of the button is clear to assistive devices.
    • also, try using the blockquote tag instead of the p for advice, to make the html semantic

    Happy coding ✨

    Marked as helpful

    0
  • Faris P 2,810

    @FarisPalayi

    Posted

    Hey, nice job on this one 👌. Some of my suggestions are:

    • You are using div everywhere. Try using semantic HTML elements like h1, h2, p, button, main, etc. This article is a good introduction to semantic HTML and HTML accessibility in general, HTML: A good basis for accessibility - MDN. It is a long read, but, it'll be worth it. I recommend you check it out if you have the time. It is such an important topic to learn in my opinion.
    • Also, the report section in this page can help you find some of the common HTML and accessibility issues with the site.

    Happy learning and coding. Cheers!

    Marked as helpful

    0
  • P
    AK 6,700

    @skyv26

    Submitted

    Hi! Really good and easy challenge, I tried to make it pixel perfect but I am happy with final output and I would like to get feedback, suggestion for any kind of improvement. Even I made it accessible. Let me know what you think

    Faris P 2,810

    @FarisPalayi

    Posted

    Nicely done 👌

    In addition to what @minimalsm said, my suggestion would be to make the toggle button accessible for keyboard users. Because, currently, the toggle button is not focusable.

    For that, instead of using dispaly: none; to hide the checkbox from the screen, I'd suggest using something like opacity: 0; height: 1px; width: 1px; (like an .sr-only class). So that the checkbox is still there in the DOM, but users won't be able to see it. Then you can simply add the focus styles when the checkbox is focused. For eg: .checkbox:focus ~ .toggle-btn { outline: solid 2px white; }. You might need to change your markup a bit for this to work, though.

    Hope it's all understandable :)

    Marked as helpful

    3
  • @Pierpaolo01

    Submitted

    Hey guys,

    In this challenge my biggest struggle (which I still haven't quite figured out) is to make the circle timer border count down dynamically with the timer, if anyone has any suggestions or can point me to an artical that would be much appriciated !!

  • @erelropeta

    Submitted

    I'm not sure if I have used CSS flexbox effectively on this. I would like to hear your thoughts on how I can improve on it. And if you could also provide feedback in naming classes/ids. I'm trying to use BEM and I'm not sure if I'm using it okay.

    Hope to hear from you! :)

    Faris P 2,810

    @FarisPalayi

    Posted

    Hey @erelita, good job on this one 👌. The site is responding well to different screen sizes.

    A few suggestions from me are:

    • The alt tag for the icons should be empty, since it doesn't provide any context or extra information; so that assistive technologies can ignore the image.
    • I think you've used the BEM methodology correctly. One thing I noticed is that, card-container__border border--one classes should be card-container__border card-container__border--one take a look here. But, BEM is flexible, so you can use it any way that works for you. So, it's not a hard rule.(also if it is meant to be reusable throughout the site, then it's not a problem) Here's a BEM cheat sheet website that can be used for reference. There are a bunch of good articles' link as well.
    • Apart from these two minor things, everything looks excellent.

    That's all from me. Have fun coding ✨

    Marked as helpful

    2
  • Alucard 870

    @Alucard2169

    Submitted

    Submitted this again, had a bit of a problem with css file names.

    ANY KIND OF CRITICISM OR FEEDBACK WOULD BE APPRECIATED

    Faris P 2,810

    @FarisPalayi

    Posted

    Good job completing this challenge. Here are a few suggestions:

    • Currently, the site looks good on 375px and 1440px. But, in between, things are not really responsive. For example in tablets. Try to make it responsive to those devices as well if you can.
    • Add cursor: pointer to "learn more" button. Though usually it'll be a link(anchor tag) to other sites.
    • One other thing you could improve in terms of accessibility is that, try to set the alt attribute in the car images to empty. So that assistive devices can ignore it, because the images doesn't really provide any context or information here.

    That's all from me. Have fun completing challenges ✨

    Marked as helpful

    1
  • @mjmorrison10

    Submitted

    How would I add the bottom section to the share button?

    I'm thinking adding another div, position absolute and putting it in place, rotating it 45 degrees, but there has to be an easier way, right?

    Working on mobile design today

    Faris P 2,810

    @FarisPalayi

    Posted

    When creating shapes using CSS, this is my reference article The Shapes of CSS

    0
  • Faris P 2,810

    @FarisPalayi

    Posted

    Looks really good on desktop. Try to make it responsive to mobile as well, if you can :)

    Marked as helpful

    0
  • Faris P 2,810

    @FarisPalayi

    Posted

    Good job on this one 👍

    The API keys should be hidden. So, if you can, try to do that. I guess that you are not using a paid API service, so it'll not be a big problem. The worst that could happen is that, your site will be broken and non-usable (I think 🤔 that's the worst).

    But, whenever using a paid API, API keys must be hidden. Because, it can lead to losing your money from your account. And it is really easy to do the exploitation if you know the API key. If I need to break your site now, all I need to do is create a for loop with a lot of requests to that API with your API key. Every API service will have a limit on how many requests can be done. What they do after the requests exceed the quota, depends on the API service. (Some ban the account. Some generates an error message, and any requests will be failed until the request rate fall under the limit, etc.).

    There are multiple methods to hide API keys. For example, by using environment variables or using serverless functions. (you can just google it to see how to do that).

    Also, if you are gonna hide this site's API key, first you need to change it, because: 1 - It is exposed to the whole world. 2 - Even if no one saw this, it can be accessed via history since it is already uploaded to GitHub.

    Nowadays, major API service providers do a lot of work to minimize this kind of damage by using a lot of methods like Principle of The Least Privilege and stuff like that. But, it's always good to hide/secure your API keys. And it is a must when you are using paid APIs.

    Hope all of this made sense. And also, don't feel pressured to do anything just because I said so :).

    Have fun coding ✨

    Marked as helpful

    2
  • Faris P 2,810

    @FarisPalayi

    Posted

    Like @nmorajda pointed out, http://localhost:3000/tracking won't work. It is because localhost only works on the computer that runs the code. i.e. it won't work when you host it.

    Here, you don't need to use localhost. You can simply fetch the json file by using the fetch api to do the job. json-server package is unnecessary here because it is only/mostly used for creating REST api endpoints for quick prototyping and mocking, so that you don't have to write server side code only to create api endpoints.

    You can just rewrite the getData function in to this:

    const API = "db.json";
    fetch(API)
         .then(response => response.json())
         .then(({ tracking }) => callback(tracking));
    

    If you were trying to learn how to host server-side code, and used json-server because you don't know backend, then go ahead and host it on Heroku or Google App Engine or any other platform you like. Just make sure that, you replace the http://localhost:3000/tracking with the url that you have given by the platform (for example: http://project-name.herokuapp.com/tracking). And also don't forget to add the start command. If you are using heroku, you can specify that in the Procfile. If you are using GAE, you will need to specify the runtime also(for eg: nodejs14), in the app.yaml file, and it will automatically look for the start command in the package.json and runs it. Hope it all make sense. If not, feel free to ask me for clarification.

    • Also, you don't need to upload(to github) or track node_modules folder. Because, as long as the packages are listed on the package.json, anyone can use npm install command to install all the packages. To tell git to not track, add node_modules/ to .gitignore file.

    Marked as helpful

    1
  • Faris P 2,810

    @FarisPalayi

    Posted

    I'm seeing this on my mobile, and one issue I found is that I can't see anything in the menu after clicking the hamburger menu. Except everything looks clean and superb👌

    1
  • @DrZubby

    Submitted

    Update: After much research effort, I was able to solve most of the initial problems I encountered (especially with responsiveness) while solving this challenge.

    I still need to be proficient with using regex for form validations though. I would appreciate suggestions that recommend very specific resources to help me improve.

    Thanks.

    Faris P 2,810

    @FarisPalayi

    Posted

    The problem here lies on the background-position property. For example, give it a value like center right 7px to always make it center on the vertical axis, and 7px far from the right.

    Marked as helpful

    1
  • Carlos 1,110

    @Carlos-A-P

    Submitted

    Wondering if I'm adding the font family correctly in CSS and HTML? I searched the font given but it still looks a little off.

    Faris P 2,810

    @FarisPalayi

    Posted

    I think that's because, the font-family of the .stat should be Inter and .statText should be Lexend Deca

    Marked as helpful

    0
  • @bishalmallick

    Submitted

    PLEASE PROVIDE SOME FEEDbACK ON MY JAVASCRIPT... IT WILL bE VERY HELPFUL TO HAVE YOUR INSIGHTS... AND PLEASE SUGGEST ME A WAY TO CHANGE THE COLOR OF THE PLACEHOLDER WHN AN ERROR IN THE EMAIL OCCURS...

    Faris P 2,810

    @FarisPalayi

    Posted

    You can use the ::placeholder CSS pseudo-element to change the color of a placeholder in CSS

    ::-webkit-input-placeholder { color: red } // 👈for webkit browsers like chrome
    ::placeholder { color: red }
    

    Since you can't really select pseudo-elements using JavaScript, create a class. Like this:

    .placeholder-error::-webkit-input-placeholder { color: red }
    

    Then, using JavaScript:

    email.classList("placeholder-error")
    

    Hope it helps😀

    0
  • @Oscar-Amarilla

    Submitted

    Greetings

    I had a hard time solving this challenge, specifically with the image. Any comment is very welcome and will be very appreciated.

    Faris P 2,810

    @FarisPalayi

    Posted

    Give your main a 100% height to solve the image being small on the mobile devices. Have fun coding✨

    Marked as helpful

    1
  • Faris P 2,810

    @FarisPalayi

    Posted

    Nice job completing this challenge 👍

    My one suggestion is that, set the alt text of the background images to empty alt="" since it's purely decorative, and so that assistive technologies can ignore that.

    Have fun coding ✨

    Marked as helpful

    1
  • Faris P 2,810

    @FarisPalayi

    Posted

    Looks good. Try to add a mobile version if you can. Related article

    Marked as helpful

    1
  • Faris P 2,810

    @FarisPalayi

    Posted

    Looks like it's working fine and looks good, I don't know React, so I can't give feedback on that part. Though there are some general accessibility improvements you can make, like:

    • Add cursor: pointer to interactive elements like buttons.
    • Add :hover, :focus and :active styles if you can.
    • Add aria-label or title or screen reader only text to buttons that have no text.
    • There's a horizontal scrollbar, try to remove it.
    • Try to clear those issues in the report section as well if you can.

    That's all from me. Have fun coding ✨

    Marked as helpful

    0
  • Faris P 2,810

    @FarisPalayi

    Posted

    The site responds well and looks nice, and it looks like you've followed good semantics and accessibility practices. So excellent job on this one 👍 One very minor issue that I noticed is that the <p> text doesn't have enough contrast ratio(you can check that by using devtool's accessiblity tab). Try to fix it if you can, and have fun coding ✨

    0
  • @nameetrajore

    Submitted

    This is my second responsive element. It is a 3 column preview card which I made using grid. I would love to hear feedback in any form.

    Faris P 2,810

    @FarisPalayi

    Posted

    Looks great and responds really well 👏

    A few suggestions are:

    • Add a :focus state style to interactive elements like buttons, since it can improve the keyboard user's experience dramatically.
    • Also, in the mobile version of the site, text is too small to read, give it a bit big font-size. Also, try to use rem or em instead of pixels for font-sizes

    That's all from me. Have fun coding 👍

    Marked as helpful

    0
  • Faris P 2,810

    @FarisPalayi

    Posted

    Works well, but do not ever set ouline: none interactive elements like buttons, unless you have a custom a :focus state style. Even if you do have that, it's a better idea to use outline: transparent taking windows contrast user's accessibility in to consideration. And try to clear those issues in the report section, like the hierarchical order of heading tags. ANd have fun coding ✨

    Marked as helpful

    0
  • atanasov36 580

    @AtanasovCode

    Submitted

    Simple but good looking design, had no problems whatsoever designing it.

    If you happen to notice any design flaws or maybe a way for me to improve my code or make it better than please share it with me :) I would love to improve and I would greatly appreciate it!

    Faris P 2,810

    @FarisPalayi

    Posted

    Some of my suggestion are:

    • Add cursor: pointer to buttons to better indicate interactivity.
    • Try to add :hover and :focus state styles for interactive elements.
    • Also, in the mobile version of the site, the card leaner than it needs to be, and some contents are overflowing from the container. It is happening because of the margins in the body, which you are using to center the card in the screen. My suggestion is that, remove those margins and use flexbox or grid to center the card. Take a look at this quick video

    Have fun coding ✨

    Marked as helpful

    1
  • @RossCurry

    Submitted

    I guess a question would be how to best organize the rows and columns to dynamically accept new content? I've seen "grid-template-rows: repeat(auto-fill, auto);" suggested but when I implemented it I got inconsistent results.

    Also, I think my border around the clip-path was a little hacky. If anyone know something a little more technical I would be greatly obliged.

    Thanks

    Faris P 2,810

    @FarisPalayi

    Posted

    clip-path seems a good idea (I don't know if it has good browser support). You can also set the .img-border's z-index to -1 and use border-radius: 50% on the image.

    Also, your desktop and mobile versions looks great, but, in the tablet version, there is a horizontal scrollbar. Moreover, take a look at HTML semantics, it leads to more consistent and readable code, also better for accessibility.

    And, the line-height property should be unitless, and using rem or em is a better idea for font-sizes.

    That's all from me. Have fun coding ✨

    Marked as helpful

    0