feat: reverse courses list for easier access to newer courses #158
Conversation
|
Hi @laporchen, what you think about the review comment. I can take over this issue if you're not available |
Which review comment you are talking about here. I don't think I see one in this PR? |
src/pages/courses/index.vue
Outdated
| @@ -44,7 +44,7 @@ const rolesCanCreateCourse = [UserRole.Admin, UserRole.Teacher]; | |||
| </tr> | |||
| </thead> | |||
| <tbody> | |||
| <tr v-for="{ course, teacher } in courses" :key="course" class="hover"> | |||
| <tr v-for="{ course, teacher } in (courses ?? []).reverse()" :key="course" class="hover"> | |||
There was a problem hiding this comment.
I feel like we should use Array.prototype.toReversed() to avoid mutating the list. How do you think?
Optionally, we can have a toggle to decide the order (new->old OR old->new), and store this preference in local storage, but overall I think reversing course list is great and will be appreciated by most of the users. So it's optional
There was a problem hiding this comment.
I prefer keeping only one order method just for now for simplicity, and I'll change toReversed to prevent potential issue in the future.
There was a problem hiding this comment.
I'm using a destructing array to prevent mutation to the original list, since we can't use es2023 stuff now.
Deploying nfe with
|
| Latest commit: |
e45491a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c258728e.nfe.pages.dev |
| Branch Preview URL: | https://feat-reverse-courses-list-or.nfe.pages.dev |
This is just a personal preference. Since most users would only use the newest courses (probably)