Skip to content
Submitted almost 4 years ago

github user search - react , sass theming

Chamu•13,820
@ChamuMutezva
A solution to the GitHub user search app challenge
Preview (opens in new tab)Code (opens in new tab)

Solution retrospective

Another exciting challenge, let me know what you think i can improve. I used react for the challenge. I wanted the card to be a component on it's own getting its data through props from the the main parent. The data comes from an api and saved it using the state hook - the value of the state is the one that was received by the card component. But for some reasons that i still need to investigate the received data in the card was always undefined. I guess it is more to do with promises but if anyone can assist me to get around the issue .

Code

Please log in to post a comment

Log in

Community feedback

  • Raymart Pamplona•16,040
    @pikapikamart

    This is the code in there.

    const date = new Date(user.created_at)
const dateJoined = date.getDate()
const year = date.getFullYear()
const month = months[date.getMonth()]
console.log(year)
console.log(month)
console.log(dateJoined)

    Basically, you are getting undefined and NaN is because you are accessing the user.created_at from the useState value and since the useEffect that holds the setState hasn't fired yet, the data will not be stored immediately. That is why at first run, you will get those results of undefined and NaN

    So the best answer to prevent this is that, have a separate component that holds the whole card. One is your Main component, the other one is Card component. Then on your Main component, you set all the datas in there, then only use:

    const Main = props =>{
  const [ user, setUser ] = useState({});
  .....
 insert useEffect to fetch data, convert it to async/await
  ........
   .....

 if ( user ) {  # only run this when user has the required Datas
  return <Card user={user} />
  }
}

    This way, the Card component will only run if the user has loaded. The Card component will all have the data usage and you won't get any falsy values.

    Marked as helpful
  • tediko•6,700
    @tediko

    Hello, Chamu! 👋

    Congrats on finishing another challenge! 🎉 Your solution looks very good and also responds well. Here's some feedback:

    1. You left a lot of console.log methods in your code. Get rid of this as it makes the code harder to read.
    2. A common convention which is widely followed is that component should follow PascalCase naming convention. The name of the folder/filename will always reflect what is present in that file that's why it is good to also use PascalCase for them, not only function within that file. e.g. (Header/Header.js)
    3. You should avoid using document.querySelector with React. There is only few cases (sometimes there is still a need for manipulating elements directly) where it can be useful but in most cases there is better way to approach that. Instead you can use useRef() hook. Refs are more reliable for getting the specific DOM node you care about. document.querySelector can give unexpected results if there are multiple elements that match the query and if you're writing a component that will show up multiple times in many places this problem will occur, that's why refs become more of a clear-cut winner.
    4. Take a look at themeControl function in header.js component. You used document.querySelector for three elements which is completely unnecessary. As you know React provides a way to inject javascript right into our HTML using something called JSX interpolation. So instead of taking element from DOM with querySelector and then inject some data with .innerHTML or .src methods you should simply create a expression inside your HTML elemetns like so:
    /* If theme is true return "Dark" if not return "Light". */
<span className="mode__state">{theme ? "Dark" : "Light"}</span>

/* If theme is true return Moon element if not return Sun element. */
<img className="mode__img" src={theme ? Moon : Sun} alt="" />
    • To completely get rid of querySelector from this function we can use document.body property. It represents the <body> node of the current document. Along with this, I used conditional (ternary) operator to either add or remove theme-dark class. Now themeControl() look like this:
     const themeControl = () => {
    theme
      ? document.body.classList.add("theme-dark")
      : document.body.classList.remove("theme-dark");
  };
    • Now you can remove themeControl() call from handleClick since you want this to perform everytime your theme change. For this purpose in useEffect with themeControl function, add theme to dependency array. The dependency array in useEffect lets you specify the conditions to trigger it. So in our case we'll watch for every theme change and trigger useEffect only when that value change, like so:
    useEffect(() => {
    themeControl();
}, [theme]);
    1. I made a small refactor of your localStorage save option in header.js. In functional programming it is good to break up your code into small reusable functions. For this, we will create two functions, one to save data to localStorage and one for retrieving that data.
    • Let's start with postThemeToLocalStorage function where we will set our newTheme value to globalTheme key. Note we don't need to include the window property. You can access built-in properties of the window object without having to type window. prefix. The window object is the top-level object of the object hierarchy. As such, whenever an object method or property is referenced in a script without the object name and dot prefix it is assumed by JavaScript to be a member of the window object. So our function will look like this:
    const postThemeToLocalStorage = (newTheme) => {
	localStorage.setItem("globalTheme", newTheme);
};
    • Then let's create getThemeFromLocalStorage function where we want to return our globalTheme key value from localStorage. You don't need to use JSON.parse() method since our value from localStorage is a string, not an object. As the localStorage value cannot be saved as boolean we want to simply convert it. For this, we make a simple comparison, to return either true or false but as boolean values, like so:
    const getThemeFromLocalStorage = () => {
    return localStorage.getItem("globalTheme") === "true";
};
    • Now, we have to clean up the code a little since we have a few unnecessary things here. Remove localStorage and savedTheme variables. Then, in handleClick function, instead of manually save our data to localStorage, use postThemeToLocalStorage(!theme). Then, add useEffect hook, that will only trigger on render (by providing empty dependency array) and save our theme from localStorage to theme state, like so:
    useEffect(() => {
    setTheme(getThemeFromLocalStorage());
}, []);
    • Finally, we have to assign a starting value for our theme state. Since you don't use anything like prefers-color-scheme to detect if the user has requested a light or dark color theme, we assign theme to be false (light theme) - const [theme, setTheme] = useState(false). Previously, when your localStorage was empty you got null value from storage. null is a falsy value so it still works, but we want to avoid such cases.

    I'll send you my copy of header.js on slack just if I forgot to include here something. Good luck with that, have fun coding! 💪

    Marked as helpful
  • Raymart Pamplona•16,040
    @pikapikamart

    Let me just fork it and see if I can help with it

  • Clément Creusat•1,655
    @ccreusat

    Woo !! It's really really nice . I would make animations a bit faster. Awesome job. Especially with the hover dashed borders for accessibility :)

  • Aakash Verma•9,500
    @skyv26

    Wow! So nice and very close to original design. Wonderful work

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord