Design comparison
Solution retrospective
I would greatly appreciate your feedback!
- I used Context API to switch an active time span. Is my usage of it correct?
Community feedback
- @adityaphasuPosted over 1 year ago
Hi @semperprimum!
First of all nice solution! Identical to the design!
Passing props down from
Grid
toCard
would've been a hassle so I think managing the state for this challenge using the ContextAPI is good! and yours look good to me.I just have a few suggestions regarding those ternary ladders inside the
Card
component. While it does work it, it kind of makes it complex and less readable. One way you can do it efficiently is by using some helper functions and adding the logic for it there. Here's how I would do it:- For the Icons instead of the ladder you can do something like an object which has keys and values as the Icon components like this:
const icons = { Work: <IconWork />, Play: <IconPlay />, Study: <IconStudy />, Exercise: <IconExercise />, Social: <IconSocial />, Self Care: <IconSelfCare /> };
and then in the return you can just do this:
<Banner className={`bg-accent-${item.title.toLowerCase().split(' ').join('-')}`}> {icons[item.title]} </Banner>
this way those 13 lines of code will just be 1! Since the JSON file has titles that are capitalized that's why in the icons object you must have to use capitalized words as keys so that they match and the icons get rendered. It will work like this -> The title from JSON matches the key from the icons object and the corresponding icon associated with the key will render out. This way you won't have to check for the titles using the ternary operator.
- For the time ladders you can make a helper function like
timeFrames
which takes in thetype
state as a param from context and then return current and previous. (basically extracting that type ternary ladder from the time and last p element in the card element since you can see they are repeated) like this:
const timeframes = (type) => { const current = item.timeframes[type].current; const previous = item.timeframes[type].previous; return { current, previous }; };
then destructure the object returned by the function while using
type
from the context:const { current, previous } = timeframes(type);
- Now
current
andprevious
will hold the values for whatever type is there. So now you can use thecurrent
like this:
<Time className="fs-600 fw-regular"> {current} hrs </Time>
and for the
previous
hours you can use it like this:<p className="fw-semi-bold text-primary-200"> {type === 'daily' ? 'Yesterday' : type === 'weekly' ? 'Last Week' : 'Last Month'} - {previous} hrs </p>
- You can probably cut down the text ternary ladder too but at this point I feel like this is enough hahahha!
- If you wanna still do it then you can do it the same way as we did for icons that is to make an object and then give key value pairs like
daily: 'Yesterday'
and so on and then the paragraph element can be rewritten like this:
<p className="fw-semi-bold text-primary-200"> {nameofobject[type]} - {previous} hrs </p>
I don't know if my explanation is okay because I'm very bad at giving names to functions but hoping you at least get the idea of what I'm trying to say haha
Keep up the hard work!
Marked as helpful1@semperprimumPosted over 1 year agoOh, thank you so much for the feedback, @adityaphasu!
I have been pondering how to replace those ugly ternary ladders. In fact, it was one of the primary questions I had regarding this project. I seem to have overlooked including it in the "questions for the community" section :/
Your explanation is truly excellent! I greatly appreciate it. Once again, thank you! I'll try refactor my code tomorrow. <3
1@adityaphasuPosted over 1 year ago@semperprimum No problem! Glad you got it ahah! I was a bit worried because I used the same/similar names for objects and function and while writing the feedback I also was confused at a point of what I was typing out lol
Also one more thing...I have no experience with using styled components but I've seen a good amount of videos on it and it seems you can separate the styled components and make another file for it and just import and use those components I think. It will make your jsx files a little bit easier to read. Again Im no expert but just telling from what I've seen xD
0@semperprimumPosted over 1 year ago@adityaphasu Yeah, I've noticed folks using that approach too. In my previous projects, I applied similar styling techniques to make reusable components for inputs, buttons, and links. I guess I could've done the same here, maybe for the buttons or something...
But, here's the thing: most of the styled-components elements I've worked with aren't exactly reusable. That's why I hesitated to create another separate file for them. It felt like it might actually make things even harder to follow, you know? Each component has its own dedicated styled wrapper and a few smaller elements, so if I were to create separate files, things might just get all jumbled up.
Anyway, I'm definitely planning to dive deeper into styled-components best practices before I kick off my next project!
0@adityaphasuPosted over 1 year ago@semperprimum Ah I meant like how people make files for CSS in react and pair them with the components and put them in a joint folder named after that component. for example in components -> card folder -> here two files which are just Card.jsx and the card.css . But I get your point and can definitely relate with managing lots of files at once. (Its actually kinda a nightmare when there are lots of folders and most are open and you just have to navigate back to the ones you wanna work on 😭).
Looking forward to your next project and good luck!!!
1
Please log in to post a comment
Log in with GitHubJoin 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