-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: ✨ create summary page #9
Conversation
P.S. I am wondering about what icon library / options are available for SvelteKit, since I didn't see any built into Skeleton |
unplugin-icons with the @iconify/json icon set if you just want access to all of the icons on this site |
@ap0nia's seems fine. I don't have an opinion, so do what you think is good @KevinWu098. Also, next time, if you're looking for a reply from me, tag me. I don't just go through every PR's comments regularly lol. |
Haha will do; I didn't want to tag ya during finals week, since an icon set wasn't too pressing |
Thanks for the consideration, but just tag me 👍. If I'm busy, I'll leave it for later. |
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.
This looks great! There are just a few things to be ironed out, in addition to the comments:
- Instead of clicking on the card taking you to the meeting page, I think only the meeting name should do that. It's too easy to fat-finger right now, I'd say. Also, we might want the location to link to something in the future.
- On mobile, the meeting card stays highlighted after toggling attendance. The previous point would fix this I'd assume.
- The Scheduled/Unscheduled tabs are misplaced on desktop. Desktop is not a priority, but this should be an easy fix.
- Similarly, the groups carousel should display a small side scrollbar on desktop.
Also, for easier integration, can you create a store for the meetings and groups and inject the boilerplate from there?
All in all, good work!
Aye aye, will do! |
@KevinWu098 Although I did say that we should communicate frequently, you don't need to say that you'll address review comments lol. Thanks for the enthusiasm though. |
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.
Nice work!
_ _
__|_|__|_|__
_|____________|__
|o o o o o o o o /
~'`~'`~'`~'`~'`~'`~'`~
Summary
Desktop:
Mobile:
Issues
Closes #6
Future Followup