-
Notifications
You must be signed in to change notification settings - Fork 3
Kommite märke på koimte sidan för komiteer #179
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
base: main
Are you sure you want to change the base?
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.
Pull request overview
This pull request adds committee badges (avatars) to committee pages on chalmers.it. The implementation creates a new GroupAvatar component that displays super group avatars using the Gamma API, following the existing pattern for displaying group-related images.
- Adds
getSuperGroupAvatarURLmethod to GammaService for fetching super group avatar URLs - Creates new
GroupAvatarcomponent to display committee badges with fallback image support - Integrates the component into the group detail page
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/services/gammaService.ts | Adds static method to generate super group avatar URLs, following existing URL generation patterns |
| src/components/GroupAvatar/GroupAvatar.tsx | New component for displaying group avatars with debugging code that should be removed |
| src/components/GroupAvatar/GroupAvatar.module.scss | Styles for the avatar component including debugging styles that should be removed |
| src/app/[locale]/(groups)/groups/[id]/page.tsx | Integrates GroupAvatar component into group pages with conditional rendering that needs improvement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {<GroupAvatar | ||
| groupAvatarUrl={gammaGroup?.superGroup.id ? GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id) : ''} | ||
| groupName={group.prettyName} | ||
| />} |
Copilot
AI
Dec 5, 2025
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.
The fragment wrapper {...} is unnecessary here. You can directly render the GroupAvatar component without wrapping it in curly braces and a fragment.
| {<GroupAvatar | |
| groupAvatarUrl={gammaGroup?.superGroup.id ? GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id) : ''} | |
| groupName={group.prettyName} | |
| />} | |
| <GroupAvatar | |
| groupAvatarUrl={gammaGroup?.superGroup.id ? GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id) : ''} | |
| groupName={group.prettyName} | |
| /> |
| {<GroupAvatar | ||
| groupAvatarUrl={gammaGroup?.superGroup.id ? GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id) : ''} | ||
| groupName={group.prettyName} | ||
| />} |
Copilot
AI
Dec 5, 2025
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.
Passing an empty string as groupAvatarUrl when gammaGroup?.superGroup.id is falsy could cause the FallbackImage component to attempt fetching an invalid URL. Consider conditionally rendering the GroupAvatar component only when a valid superGroup.id exists, similar to how member rendering is handled above (lines 65-84).
| {<GroupAvatar | |
| groupAvatarUrl={gammaGroup?.superGroup.id ? GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id) : ''} | |
| groupName={group.prettyName} | |
| />} | |
| {gammaGroup?.superGroup.id && ( | |
| <GroupAvatar | |
| groupAvatarUrl={GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id)} | |
| groupName={group.prettyName} | |
| /> | |
| )} |
| import styles from './GroupAvatar.module.scss'; | ||
|
|
||
| const GroupAvatar = ({ | ||
| groupAvatarUrl, |
Copilot
AI
Dec 5, 2025
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.
There is trailing whitespace after the comma. Please remove it to maintain code consistency.
| groupAvatarUrl, | |
| groupAvatarUrl, |
| .url { | ||
| margin-top: 0.5rem; | ||
| font-size: 0.75rem; | ||
| color: var(--muted-color, #666); | ||
| word-break: break-all; | ||
| } |
Copilot
AI
Dec 5, 2025
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.
These styles are for debugging purposes and should be removed before merging to production.
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.
@copilot open a new pull request to apply changes based on this feedback
GAsplund
left a comment
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.
Looks good overall, I have one small change request. Check separate comment.
| {<GroupAvatar | ||
| groupAvatarUrl={gammaGroup?.superGroup.id ? GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id) : ''} | ||
| groupName={group.prettyName} | ||
| />} |
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.
| {<GroupAvatar | |
| groupAvatarUrl={gammaGroup?.superGroup.id ? GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id) : ''} | |
| groupName={group.prettyName} | |
| />} | |
| <GroupAvatar | |
| groupAvatarUrl={gammaGroup?.superGroup.id ? GammaService.getSuperGroupAvatarURL(gammaGroup.superGroup.id) : ''} | |
| groupName={group.prettyName} | |
| /> |
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.
Ehm jag tryckte på commit suggestion och nu runnar inte koden.. ;( vet inte varför
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.
Vad får du för felmeddelande?
Co-authored-by: Goostaf <[email protected]>
|
@Alex04412 I've opened a new pull request, #181, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <[email protected]>
* Initial plan * Remove debugging URL display and styles from GroupAvatar component Co-authored-by: Alex04412 <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Alex04412 <[email protected]>
Gör så att komite märken visas upp på deras komite sida på chalmers.it