-
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 group page #30
Conversation
How's progress on this? Is it ready for review? |
I still want to make some more changes, but I'll let you know by meeting |
…MeetingList components
Clicking on the meeting should always take you to the meeting setup page. |
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.
Code mostly looks fine with some suggestions.
Also, please write the PR description such that I know how to test the feature and how it's supposed to be used. E.g., "go to /groups/1
to see demo" (which gives me a server error).
The problem with the page crashing might be solved if you just merge main in.
style="background-image:url({member.profile_picture})" | ||
></div> | ||
<p | ||
class=" mt-auto line-clamp-2 max-h-12 text-left text-lg font-bold text-black hover:text-blue-500 md:text-xl" |
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.
class=" mt-auto line-clamp-2 max-h-12 text-left text-lg font-bold text-black hover:text-blue-500 md:text-xl" | |
class="mt-auto line-clamp-2 max-h-12 text-left text-lg font-bold text-black hover:text-blue-500 md:text-xl" |
groupAndSortScheduledMeetings, | ||
} from "$lib/utils/summary-helpers"; | ||
import LocationIcon from "~icons/material-symbols/location-on"; | ||
import ClockIcon from "~icons/material-symbols/nest-clock-farsight-analog-outline"; | ||
|
||
const sortedMeetings = groupAndSortScheduledMeetings($scheduledMeetings); | ||
export let groupID: number | undefined; | ||
// const modalStore = getModalStore(); |
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.
Can you remove the commented-out code? You can always go back to this PR and check the history if you want to refer to it in the future.
@@ -54,7 +71,7 @@ | |||
<RadioItem bind:group={meeting.attending} name="justify" value={"Yes"} required | |||
>Yes</RadioItem | |||
> | |||
<RadioItem bind:group={meeting.attending} name="justify" value={"No"} required | |||
<RadioItem bind:group={meeting.attending} name="justify" value={"No"} required } |
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.
<RadioItem bind:group={meeting.attending} name="justify" value={"No"} required } | |
<RadioItem bind:group={meeting.attending} name="justify" value={"No"} required |
@@ -0,0 +1,8 @@ | |||
export type Member = { |
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.
Should be called GroupMember
just to make it clearer.
]; | ||
|
||
export const members = writable<Member[]>(exampleMembers); | ||
export const attendees = writable<Member[]>(exampleMembers); |
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 is unused and should be deleted.
@@ -57,6 +57,12 @@ export function groupAndSortScheduledMeetings(scheduledMeetings: ScheduledMeetin | |||
return groupedMeetings; | |||
} | |||
|
|||
export function filterMeetingsByGroupID( |
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.
Please move this (and other relevant function(s)) to the component in which it's used, if it's only used in one.
We're going to redesign, so I'm going to close this for now. We can pull code from this for the new page when that's ready. |
Creating groups page with 1. users in group and 2. meetings involving the group