Conversation
Contributor
📝 WalkthroughWalkthroughFour new Svelte UI components for ticket feedback functionality are introduced: FeedbackBadge (displays rating badges), FeedbackForm (handles feedback submission and editing), FeedbackItem (renders individual feedback entries), and FeedbackList (displays feedback collections with empty state handling). Changes
Sequence DiagramsequenceDiagram
actor User
participant FeedbackForm
participant API as Backend API
participant Parent as Parent Component
User->>FeedbackForm: Fill rating & comments
User->>FeedbackForm: Click Submit/Update
FeedbackForm->>FeedbackForm: Validate (set saving=true)
alt Editing existing feedback
FeedbackForm->>API: updateFeedback(ticketId, payload)
else Creating new feedback
FeedbackForm->>API: createFeedback(ticketId, payload)
end
API-->>FeedbackForm: Return TicketFeedback
FeedbackForm->>FeedbackForm: Update local state
FeedbackForm->>Parent: Dispatch 'saved' event
Parent->>FeedbackForm: Feedback submitted ✓
alt Error scenario
API-->>FeedbackForm: Error response
FeedbackForm->>FeedbackForm: Capture error message
FeedbackForm->>Parent: Dispatch 'error' event
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Reviewer's GuideAdds a reusable Svelte-based feedback UI for tickets, including a form to create/update feedback, a display item, a list wrapper, and a rating badge, all wired to existing API types and helpers. Sequence diagram for ticket feedback creation and update flowsequenceDiagram
actor User
participant FeedbackForm
participant FeedbackAPI
participant ParentComponent
User->>FeedbackForm: Submit feedback form
FeedbackForm->>FeedbackForm: handleSubmit(event)
FeedbackForm->>FeedbackForm: Validate disabled and saving flags
alt Existing feedback
FeedbackForm->>FeedbackAPI: updateFeedback(ticketId, feedbackId, payload)
FeedbackAPI-->>FeedbackForm: TicketFeedback
else New feedback
FeedbackForm->>FeedbackAPI: createFeedback(ticketId, payload)
FeedbackAPI-->>FeedbackForm: TicketFeedback
end
FeedbackForm->>ParentComponent: dispatch saved(feedback)
ParentComponent-->>ParentComponent: Update ticket feedback state
rect rgb(255,230,230)
User->>FeedbackForm: Submit when API fails
FeedbackForm->>FeedbackAPI: createFeedback or updateFeedback
FeedbackAPI-->>FeedbackForm: Error
FeedbackForm->>ParentComponent: dispatch error(error)
end
User->>FeedbackForm: Click cancel
FeedbackForm->>ParentComponent: dispatch cancel()
Class diagram for new Svelte feedback componentsclassDiagram
class FeedbackForm {
+number ticketId
+TicketFeedback existing
+boolean disabled
-number rating
-string comments
-boolean saving
-string error
+handleSubmit(e Event) void
}
class FeedbackItem {
+TicketFeedback feedback
+boolean compact
}
class FeedbackList {
+TicketFeedback[] feedbacks
+string emptyMessage
}
class FeedbackBadge {
+number rating
+string size
}
class TicketFeedback {
+number id
+number rating
+string comments
+string created_at
+Student student
}
class Student {
+string name
+string email
}
class FeedbackCreatePayload {
+number rating
+string comments
}
class FeedbackUpdatePayload {
+number rating
+string comments
}
class FeedbackAPI {
+createFeedback(ticketId number, payload FeedbackCreatePayload) TicketFeedback
+updateFeedback(ticketId number, feedbackId number, payload FeedbackUpdatePayload) TicketFeedback
}
FeedbackList "*" --> "1" FeedbackItem : renders
FeedbackItem "1" --> "1" TicketFeedback : displays
TicketFeedback "0..1" --> "1" Student : student
FeedbackForm --> FeedbackAPI : uses
FeedbackForm --> FeedbackCreatePayload : builds
FeedbackForm --> FeedbackUpdatePayload : builds
FeedbackBadge --> TicketFeedback : displays rating
FeedbackList --> FeedbackBadge : can_use_for_summary
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
labelelements forratingandcommentsreferenceforattributes, but the corresponding<select>and<textarea>lack matchingids, which breaks the association and could be improved for accessibility. - In
FeedbackForm.sveltethe submit handler both useson:submit|preventDefaultin the template and callse.preventDefault()inhandleSubmit, so you can remove one of these to avoid redundancy. - Consider disabling the rating and comments inputs while
saving(e.g.disabled={disabled || saving}) so the UI state is consistent and users can't change values mid‑submission.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `label` elements for `rating` and `comments` reference `for` attributes, but the corresponding `<select>` and `<textarea>` lack matching `id`s, which breaks the association and could be improved for accessibility.
- In `FeedbackForm.svelte` the submit handler both uses `on:submit|preventDefault` in the template and calls `e.preventDefault()` in `handleSubmit`, so you can remove one of these to avoid redundancy.
- Consider disabling the rating and comments inputs while `saving` (e.g. `disabled={disabled || saving}`) so the UI state is consistent and users can't change values mid‑submission.
## Individual Comments
### Comment 1
<location> `frontend/src/components/ui/feedback/FeedbackItem.svelte:21-23` </location>
<code_context>
+ <p class="text-sm text-base-content/60 mt-2">{feedback.comments}</p>
+ {/if}
+
+ {#if !compact}
+ <div class="text-[11px] text-base-content/40 mt-2">By {feedback.student?.name ?? feedback.student?.email}</div>
+ {/if}
+ </div>
</code_context>
<issue_to_address>
**suggestion:** The author line can render `undefined` if both name and email are absent.
If `feedback.student` exists but both `name` and `email` are `null`/`undefined`, this will render `By undefined`. Consider adding a final fallback or skipping the line when no identifier is available, e.g. `{#if feedback.student?.name || feedback.student?.email}By {feedback.student?.name ?? feedback.student?.email}{/if}`.
```suggestion
{#if !compact && (feedback.student?.name || feedback.student?.email)}
<div class="text-[11px] text-base-content/40 mt-2">
By {feedback.student?.name ?? feedback.student?.email}
</div>
{/if}
```
</issue_to_address>
### Comment 2
<location> `frontend/src/components/ui/feedback/FeedbackList.svelte:11-12` </location>
<code_context>
+
+{#if feedbacks?.length}
+ <div class="flex flex-col gap-3">
+ {#each feedbacks as f}
+ <FeedbackItem feedback={f} />
+ {/each}
+ </div>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The `each` block lacks a key, which can lead to less efficient updates and potential UI glitches.
If `TicketFeedback` has a stable identifier (e.g. `id`), consider using a keyed each block like `{#each feedbacks as f (f.id)}` so DOM updates stay aligned when items are added, removed, or reordered.
Suggested implementation:
```
<div class="flex flex-col gap-3">
{#each feedbacks as f (f.id)}
<FeedbackItem feedback={f} />
{/each}
</div>
```
This assumes that `TicketFeedback` has a stable `id` field. If the identifier is named differently (e.g. `feedbackId` or `uuid`), replace `f.id` with the appropriate property (e.g. `f.feedbackId`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+21
to
+23
| {#if !compact} | ||
| <div class="text-[11px] text-base-content/40 mt-2">By {feedback.student?.name ?? feedback.student?.email}</div> | ||
| {/if} |
Contributor
There was a problem hiding this comment.
suggestion: The author line can render undefined if both name and email are absent.
If feedback.student exists but both name and email are null/undefined, this will render By undefined. Consider adding a final fallback or skipping the line when no identifier is available, e.g. {#if feedback.student?.name || feedback.student?.email}By {feedback.student?.name ?? feedback.student?.email}{/if}.
Suggested change
| {#if !compact} | |
| <div class="text-[11px] text-base-content/40 mt-2">By {feedback.student?.name ?? feedback.student?.email}</div> | |
| {/if} | |
| {#if !compact && (feedback.student?.name || feedback.student?.email)} | |
| <div class="text-[11px] text-base-content/40 mt-2"> | |
| By {feedback.student?.name ?? feedback.student?.email} | |
| </div> | |
| {/if} |
Comment on lines
+11
to
+12
| {#each feedbacks as f} | ||
| <FeedbackItem feedback={f} /> |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): The each block lacks a key, which can lead to less efficient updates and potential UI glitches.
If TicketFeedback has a stable identifier (e.g. id), consider using a keyed each block like {#each feedbacks as f (f.id)} so DOM updates stay aligned when items are added, removed, or reordered.
Suggested implementation:
<div class="flex flex-col gap-3">
{#each feedbacks as f (f.id)}
<FeedbackItem feedback={f} />
{/each}
</div>
This assumes that TicketFeedback has a stable id field. If the identifier is named differently (e.g. feedbackId or uuid), replace f.id with the appropriate property (e.g. f.feedbackId).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
frontend/src/components/ui/feedback/FeedbackList.svelte (1)
9-13: Add a key to the#eachblock and remove the redundant optional chain.
feedbacks?.length—feedbacksalready defaults to[], so?.is unnecessary.- The
#eachblock is missing a key expression. Without a key, Svelte re-uses DOM nodes by index, which produces incorrect diffs when items are reordered or removed mid-list.♻️ Proposed fix
-{`#if` feedbacks?.length} +{`#if` feedbacks.length} <div class="flex flex-col gap-3"> - {`#each` feedbacks as f} + {`#each` feedbacks as f (f.id)} <FeedbackItem feedback={f} /> {/each} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ui/feedback/FeedbackList.svelte` around lines 9 - 13, Remove the redundant optional chain in the if-check (use feedbacks.length since feedbacks defaults to []), and add a stable key to the Svelte `#each` block to prevent DOM reuse issues — update the each that iterates over feedbacks as f to include a key expression using a unique identifier from the feedback object (e.g., f.id or f._id) and ensure the FeedbackItem component still receives feedback={f}.frontend/src/components/ui/feedback/FeedbackForm.svelte (1)
2-2:createEventDispatcher,on:directives, andexport letare all deprecated in Svelte 5.The official Svelte 5 docs mark
createEventDispatcheras deprecated, recommending "callback props and/or the$host()rune instead." By deprecatingcreateEventDispatcherand theon:directive in favor of callback props and normal element properties, Svelte 5 improves type safety. The$propsrune has replaced theexport letsyntax, allowing props to be accessed in a more concise and TypeScript-friendly way.This applies across all four new components in this PR, but
FeedbackFormis the most impactful because it relies on the dispatcher for"saved","error", and"cancel"events.♻️ Suggested Svelte 5 migration for FeedbackForm
<script lang="ts"> - import { createEventDispatcher } from "svelte"; import type { FeedbackCreatePayload, FeedbackUpdatePayload, TicketFeedback } from "../../../types/feedback.ts"; import { createFeedback, updateFeedback } from "../../../lib/api/feedback.ts"; - export let ticketId: number; - export let existing: TicketFeedback | null = null; - export let disabled: boolean = false; - - const dispatch = createEventDispatcher(); - - let rating: number = existing?.rating ?? 5; - let comments: string = existing?.comments ?? ""; - let saving = false; - let error: string | null = null; + let { + ticketId, + existing = null, + disabled = false, + onsaved, + onerror, + oncancel, + }: { + ticketId: number; + existing?: TicketFeedback | null; + disabled?: boolean; + onsaved?: (e: { feedback: TicketFeedback }) => void; + onerror?: (e: { error: string }) => void; + oncancel?: () => void; + } = $props(); + + let rating = $state(existing?.rating ?? 5); + let comments = $state(existing?.comments ?? ""); + let saving = $state(false); + let error = $state<string | null>(null);Replace all
dispatch(...)calls withonsaved?.(...),onerror?.(...),oncancel?.()respectively, and changeon:click→onclick,on:submit→onsubmit.Also applies to: 10-10, 46-46, 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ui/feedback/FeedbackForm.svelte` at line 2, FeedbackForm uses deprecated Svelte 5 APIs (createEventDispatcher, dispatch calls, on: directives, and export let); update the component to accept callback props via $props (or named callback props) and replace dispatch("saved"/"error"/"cancel") with calling onsaved?.(...), onerror?.(...), and oncancel?.() respectively, and change template event bindings from on:click → onclick and on:submit → onsubmit; also remove createEventDispatcher usage and migrate any exported props from export let to the $props rune or equivalent Svelte 5 prop syntax so the component receives the callback props.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ui/feedback/FeedbackBadge.svelte`:
- Around line 11-12: The "No rating" else branch in FeedbackBadge.svelte
hardcodes the CSS class badge-sm, ignoring the exported size prop; update the
else branch to use the size prop when building the badge class (e.g., derive
badge-{size} or use a conditional/class binding based on the exported size
variable) so the div uses the same size logic as the rated branch and reflects
size="md" or other values.
In `@frontend/src/components/ui/feedback/FeedbackForm.svelte`:
- Around line 48-49: The labels reference ids that don't exist, breaking
label-control association; add id="rating" to the <select> element that binds
value to rating (bind:value={rating}) and add id="comments" to the <textarea>
that binds value to comments (bind:value={comments}), ensuring the label
for="rating" and label for="comments" correctly target those controls so
clicking labels and screen readers work as intended.
- Line 49: The select element bound to rating (bind:value={rating}) uses the
wrong DaisyUI classes ("input input-sm ...") which misstyles dropdowns; update
the class attribute on that <select> (the element with bind:value={rating} and
disabled={disabled}) to use "select select-sm" and preserve the sizing utility
(e.g., "select select-sm max-w-[84px]") so the dropdown arrow and padding render
correctly.
In `@frontend/src/components/ui/feedback/FeedbackItem.svelte`:
- Line 22: The "By ..." line can render empty if feedback.student exists but
both name and email are missing; update the expression in FeedbackItem.svelte
(the div with class "text-[11px]...") to include a final fallback value (e.g.,
'Unknown' or 'Unknown student') so it reads something like
feedback.student?.name ?? feedback.student?.email ?? 'Unknown' to guarantee a
non-empty display when neither field is present.
---
Nitpick comments:
In `@frontend/src/components/ui/feedback/FeedbackForm.svelte`:
- Line 2: FeedbackForm uses deprecated Svelte 5 APIs (createEventDispatcher,
dispatch calls, on: directives, and export let); update the component to accept
callback props via $props (or named callback props) and replace
dispatch("saved"/"error"/"cancel") with calling onsaved?.(...), onerror?.(...),
and oncancel?.() respectively, and change template event bindings from on:click
→ onclick and on:submit → onsubmit; also remove createEventDispatcher usage and
migrate any exported props from export let to the $props rune or equivalent
Svelte 5 prop syntax so the component receives the callback props.
In `@frontend/src/components/ui/feedback/FeedbackList.svelte`:
- Around line 9-13: Remove the redundant optional chain in the if-check (use
feedbacks.length since feedbacks defaults to []), and add a stable key to the
Svelte `#each` block to prevent DOM reuse issues — update the each that iterates
over feedbacks as f to include a key expression using a unique identifier from
the feedback object (e.g., f.id or f._id) and ensure the FeedbackItem component
still receives feedback={f}.
Comment on lines
+11
to
+12
| {:else} | ||
| <div class="badge badge-ghost badge-sm text-base-content/40">No rating</div> |
Contributor
There was a problem hiding this comment.
size prop is ignored in the "No rating" branch.
The {:else} branch always renders badge-sm regardless of the size prop, creating inconsistent sizing when the parent passes size="md".
🐛 Proposed fix
- <div class="badge badge-ghost badge-sm text-base-content/40">No rating</div>
+ <div class={"badge badge-ghost text-base-content/40 " + (size === 'sm' ? 'badge-sm' : 'badge-md')}>No rating</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {:else} | |
| <div class="badge badge-ghost badge-sm text-base-content/40">No rating</div> | |
| {:else} | |
| <div class={"badge badge-ghost text-base-content/40 " + (size === 'sm' ? 'badge-sm' : 'badge-md')}>No rating</div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ui/feedback/FeedbackBadge.svelte` around lines 11 -
12, The "No rating" else branch in FeedbackBadge.svelte hardcodes the CSS class
badge-sm, ignoring the exported size prop; update the else branch to use the
size prop when building the badge class (e.g., derive badge-{size} or use a
conditional/class binding based on the exported size variable) so the div uses
the same size logic as the rated branch and reflects size="md" or other values.
Comment on lines
+48
to
+49
| <label for="rating" class="text-xs text-base-content/60">Rating</label> | ||
| <select class="input input-sm max-w-[84px]" bind:value={rating} disabled={disabled}> |
Contributor
There was a problem hiding this comment.
Missing id attributes break label-input association — accessibility blocker.
Both labels declare for attributes that reference IDs which don't exist on the corresponding inputs:
<label for="rating">→<select>has noid="rating"<label for="comments">→<textarea>has noid="comments"
Clicking either label won't focus its control, and screen readers cannot programmatically associate them.
🛡️ Proposed fix
- <select class="input input-sm max-w-[84px]" bind:value={rating} disabled={disabled}>
+ <select id="rating" class="input input-sm max-w-[84px]" bind:value={rating} disabled={disabled}>- <textarea class="textarea textarea-sm w-full mt-1" rows={3} bind:value={comments} disabled={disabled}></textarea>
+ <textarea id="comments" class="textarea textarea-sm w-full mt-1" rows={3} bind:value={comments} disabled={disabled}></textarea>Also applies to: 60-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ui/feedback/FeedbackForm.svelte` around lines 48 -
49, The labels reference ids that don't exist, breaking label-control
association; add id="rating" to the <select> element that binds value to rating
(bind:value={rating}) and add id="comments" to the <textarea> that binds value
to comments (bind:value={comments}), ensuring the label for="rating" and label
for="comments" correctly target those controls so clicking labels and screen
readers work as intended.
| <form class="space-y-3" on:submit|preventDefault={handleSubmit}> | ||
| <div class="flex items-center gap-3"> | ||
| <label for="rating" class="text-xs text-base-content/60">Rating</label> | ||
| <select class="input input-sm max-w-[84px]" bind:value={rating} disabled={disabled}> |
Contributor
There was a problem hiding this comment.
Wrong DaisyUI class on <select> — use select select-sm, not input input-sm.
DaisyUI's input class targets text-type <input> elements; applying it to a <select> produces incorrect styling (misaligned padding, potentially hidden or overlapping native dropdown arrow). The correct DaisyUI classes for a styled select are select and select-sm.
🐛 Proposed fix
- <select class="input input-sm max-w-[84px]" bind:value={rating} disabled={disabled}>
+ <select id="rating" class="select select-sm max-w-[84px]" bind:value={rating} disabled={disabled}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <select class="input input-sm max-w-[84px]" bind:value={rating} disabled={disabled}> | |
| <select id="rating" class="select select-sm max-w-[84px]" bind:value={rating} disabled={disabled}> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ui/feedback/FeedbackForm.svelte` at line 49, The
select element bound to rating (bind:value={rating}) uses the wrong DaisyUI
classes ("input input-sm ...") which misstyles dropdowns; update the class
attribute on that <select> (the element with bind:value={rating} and
disabled={disabled}) to use "select select-sm" and preserve the sizing utility
(e.g., "select select-sm max-w-[84px]") so the dropdown arrow and padding render
correctly.
| {/if} | ||
|
|
||
| {#if !compact} | ||
| <div class="text-[11px] text-base-content/40 mt-2">By {feedback.student?.name ?? feedback.student?.email}</div> |
Contributor
There was a problem hiding this comment.
Silent empty output when both student.name and student.email are absent.
feedback.student?.name ?? feedback.student?.email resolves to undefined when the student object is present but has neither field populated, rendering "By " with no value. Add a final fallback.
🛡️ Proposed fix
- <div class="text-[11px] text-base-content/40 mt-2">By {feedback.student?.name ?? feedback.student?.email}</div>
+ <div class="text-[11px] text-base-content/40 mt-2">By {feedback.student?.name ?? feedback.student?.email ?? 'Unknown'}</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div class="text-[11px] text-base-content/40 mt-2">By {feedback.student?.name ?? feedback.student?.email}</div> | |
| <div class="text-[11px] text-base-content/40 mt-2">By {feedback.student?.name ?? feedback.student?.email ?? 'Unknown'}</div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ui/feedback/FeedbackItem.svelte` at line 22, The "By
..." line can render empty if feedback.student exists but both name and email
are missing; update the expression in FeedbackItem.svelte (the div with class
"text-[11px]...") to include a final fallback value (e.g., 'Unknown' or 'Unknown
student') so it reads something like feedback.student?.name ??
feedback.student?.email ?? 'Unknown' to guarantee a non-empty display when
neither field is present.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add reusable UI components for creating, displaying, and summarizing ticket feedback.
New Features:
Summary by CodeRabbit