-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
29 connect supabase #46
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯 @ilyaflaks This is amazing - A work of code art 🔥
I have a small improvement to recommend but that's it - everything else looks pristine. Thanks so much for your hard work on this, huge step forward for the project.
src/components/ChallengeTile.tsx
Outdated
<Heading size='md'>{name}</Heading> | ||
<Text | ||
color={ | ||
difficulty === 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small recommendation here for easier readability and reuse :) Since enums automatically assign themselves numeric values, it might be a good option here.
enum DifficultyLevel {
EASY,
MEDIUM,
HARD
}
console.log(DifficultyLevel.HARD) // 2
Also, for the colors, we could leverage a map to really simplify things, although this is a bit of overkill lol
const colorForDifficulty: Map<DifficultyLevel, string> = new Map([
[DifficultyLevel.EASY]: 'green.600',
...
])
// In the JSX
colorForDifficulty(difficulty)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of using enums but I'm not sure I understand how to implement it.
- the numeric values are coming from the database. Should I change 1,2,3 to EASY, MEDIUM and HARD on the Supabase table and then change the type for ChallengeProps to the enum you wrote above? On Supabase side I can create a new type as ENUM and change the 'difficulty' column type from int2 to the new one.
https://www.postgresql.org/docs/current/datatype-enum.html - as far as I know, Map objects can only be accessed with a .get() method, so it might be easier to just create a regular object and access it like so: colorForDifficulty[difficulty]. In that case, the key of the object can be a string or a number. I agree that this is a cleaner line of code than a nested ternary statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1
Hmmm that's a good question. On one hand an enum provides a better description of the difficulty level but on the other hand we could leverage numbers in the future to make something like a loading bar style display of difficulty level. I lean towards keeping the int2
in supabase and using the enum
to represent the numeric values in the frontend code. Something like this sandbox I think would work well :)
2
I agree object notation is easier, maps are good for performance but in this situation it's just unnecessary haha. I included an example of the object with enum keys up in the codesandbox :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I understand now! Thanks for creating the codesandbox. I'll keep the table types as they are and add the enum to the front end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added enums. I made them lower case because I think it looks better on the page. Uppercase looks good if the font is smaller. Let me know what you guys think!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did anyone else get kicked out of the MIT xPro Slack channel? I don't even see it on the list of my channels like it ceased to exist altogether... Should we create our own Slack channel or Discord?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was kicked too! I created a discord over in #47
Sorry I've been away for so long, back now to continue on the project!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 Thanks very much @ilyaflaks!!
Connected Supabase with a the app
Issue: #29