🎨 Enable to update submission status from drop down (#1706) #1844
Conversation
WalkthroughThis pull request replaces the modal-based submission status update with a dropdown component. A new Svelte component, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as TaskTableCell
participant D as UpdatingDropdown
participant S as Server
U->>C: Click update trigger
C->>D: Toggle dropdown display
D->>U: Render dropdown with statuses
U->>D: Select new status
D->>S: Submit update via fetch
S-->>D: Return response (success/error)
D->>C: Invoke onupdate callback with updated result
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/lib/components/TaskTables/TaskTableBodyCell.svelte (2)
15-15: Consider documenting the newonupdateprop.
Exposing an optionalonupdatecallback is a good approach, but a short docstring or comment clarifying its role for external consumers might be helpful.
55-69: Enhance accessibility for the toggle button.
While toggling the dropdown, consider adding anaria-expandedattribute to indicate whether it is open or closed for screen readers.<button type="button" class="flex-shrink-0 w-6 ml-auto" onclick={() => updatingDropdown.toggle()} - aria-label="Update submission for {selectedTaskResult.title}" + aria-label="Update submission for {selectedTaskResult.title}" + aria-expanded={dropdownStatus} > <EllipsisVertical class="w-4 h-4 mx-auto" /> </button>src/lib/components/SubmissionStatus/UpdatingDropdown.svelte (2)
70-109: Automatic submission trigger might be unexpected.
Starting the form submission after a smallsetTimeoutis convenient, but it may surprise users expecting to fill additional details. Consider showing a confirm step or providing a clear explanation of the flow.
110-142: Graceful fallback in fetch calls.
The.catch()block updates a store with an error message. This is good. However, consider adding a mechanism for repeated attempts or more detailed error logging if needed.src/lib/components/TaskTables/TaskTable.svelte (1)
71-107: Efficient approach to map tasks for quick indexing.
BuildingtaskResultsMapandtaskIndicesMapfrom the initial array is O(N). This is acceptable for moderate data sizes. If the data grows significantly, consider more advanced data structures or incremental updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/SubmissionStatus/UpdatingDropdown.svelte(1 hunks)src/lib/components/TaskTables/TaskTable.svelte(5 hunks)src/lib/components/TaskTables/TaskTableBodyCell.svelte(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: preview
🔇 Additional comments (13)
src/lib/components/TaskTables/TaskTableBodyCell.svelte (4)
7-8: New imports look correct.
The additional imports forExternalLinkWrapperandUpdatingDropdownare consistent with their usage below.
18-18: Defaulting to a no-op is helpful.
Establishing a default empty function prevents runtime errors if the prop is not provided.
20-20: Storing a local reference for the dropdown.
This reference is needed to invoketoggle(). Ensure the variable is always set when the snippet is rendered to avoid null references if conditions change.
28-28: Cleaner snippet usage.
Rendering the snippetsubmissionUpdaterAndLinksOfTaskDetailPagedirectly aligns well with the new design. The code is more readable and modular.src/lib/components/SubmissionStatus/UpdatingDropdown.svelte (6)
1-27: Usage block is clear.
These commented lines provide a self-contained usage guide and example forUpdatingDropdown. Keeping this inlined is helpful for future reference.
28-47: Prop definitions and typing.
DefiningtaskResult,isLoggedIn, andonupdateensures clarity. The defaultonupdatein the parent file corresponds well with this mandatory prop here.
49-69: Dropdown state management appears robust.
Using$state(...)to manage the dropdown status is straightforward. Thetoggle()export and the effect block align well with Svelte's reactivity.
144-151: Client-side timestamp assignment.
updated_at: new Date()may skew if the client clock differs from the server. Validate whether the server controls timestamps.
153-165:updateTaskResultfunction is straightforward.
Spreading the originaltaskResultand updating just the status fields is a clean approach. This is easy to read and maintain.
178-229: Form-based dropdown structure is logically sound.
Hiding the form ensures a smooth user experience without cluttering the UI. Theuse:enhancedirective for progressive enhancement is a solid choice.src/lib/components/TaskTables/TaskTable.svelte (3)
33-33: Inline comment is fine.
Preparing the contest table provider is well-labeled and does not introduce issues.
56-56: Body cell classes comment is unchanged but references next lines.
No problem with referencing the newly updated code that follows.
169-169: Event-driven task update looks good.
Passing theupdatedTasktohandleUpdateTaskResultensures a straightforward flow without needing a modal. This is consistent with the new dropdown approach.
close #1706
Summary by CodeRabbit
New Features
Refactor