Added confirmation prompt for late days #2249
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request focus on the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
🧹 Outside diff range and nitpick comments (3)
app/views/assessments/_handin_form.html.erb (3)
56-56: LGTM! Consider enhancing the confirmation message.The implementation correctly adds a confirmation prompt for late submissions. However, the message could be more explicit to better prevent accidental submissions.
Consider this improved message format:
-<%= f.submit("Submit Late", id: "fake-submit", class: "btn primary handin-btn disabled", data: { confirm: "You are #{grace_late_info}. Are you sure you want to continue?" }) %> +<%= f.submit("Submit Late", id: "fake-submit", class: "btn primary handin-btn disabled", data: { confirm: "Warning: You are about to make a late submission.\n\nDetails: #{grace_late_info}\n\nDo you want to proceed with this late submission?" }) %>
56-56: Consider adding ARIA attributes for accessibility.To improve accessibility, consider adding ARIA attributes to better communicate the late submission status to screen readers.
-<%= f.submit("Submit Late", id: "fake-submit", class: "btn primary handin-btn disabled", data: { confirm: "You are #{grace_late_info}. Are you sure you want to continue?" }) %> +<%= f.submit("Submit Late", id: "fake-submit", class: "btn primary handin-btn disabled", aria: { label: "Submit assignment late using #{grace_late_info}" }, data: { confirm: "You are #{grace_late_info}. Are you sure you want to continue?" }) %>
Line range hint
18-22: Consider unifying late submission warnings.Currently, there are multiple places showing late submission information:
- Warning message above the form
- Confirmation dialog
- Small text below the submit button
Consider consolidating these to provide a more cohesive user experience.
Some suggestions:
- Use consistent wording across all messages
- Consider adding a visual indicator (e.g., warning icon) next to the submit button
- Consider moving the grace day calculation logic to a helper method to ensure consistency
Also applies to: 56-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
app/views/assessments/_handin_form.html.erb(1 hunks)
🔇 Additional comments (1)
app/views/assessments/_handin_form.html.erb (1)
56-56: Verify disabled button behavior with confirmation dialog.
The button has a 'disabled' class which might interfere with the confirmation dialog. Let's verify this interaction.
✅ Verification successful
The disabled class will not interfere with the confirmation dialog
Based on the codebase analysis:
- The JavaScript in
handin.jsproperly manages thedisabledclass on the#fake-submitbutton, toggling it based on form validation state - The confirmation dialog is implemented using Rails' built-in
data: { confirm: }attribute, which works independently of the button's disabled state - Multiple examples in the codebase show similar patterns of using
data: { confirm: }with various button states, indicating this is a well-established pattern
The disabled state only affects the visual appearance and click behavior of the button, while the confirmation dialog is handled by Rails UJS at a different level. When the button becomes enabled through JavaScript, both the click handler and confirmation dialog will work as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for JavaScript that might enable/disable the submit button
rg -t js "fake-submit|handin-btn|submissionForm" --type-add 'js:*.{js,coffee}'
# Look for similar confirmation dialogs with disabled buttons
rg -t erb 'data: { confirm:.*disabled'
Length of output: 956
Script:
#!/bin/bash
# Search for confirmation dialog patterns in ERB files with better regex
rg -t erb 'data:\s*{\s*confirm:'
# Look for any JavaScript that handles confirmation dialogs
rg -t js 'confirm|data-confirm' --type-add 'js:*.{js,coffee}'
# Check for any Rails UJS or Turbolinks confirmation handlers
rg 'rails.*confirm|turbolinks.*confirm|data-confirm'
Length of output: 977
Script:
#!/bin/bash
# Search for confirmation dialog patterns in ERB files (fixed regex)
rg "data: \{ confirm:" -t erb
# Look specifically for the handin form's JavaScript behavior
rg "handin.*submit|submit.*handin" -t js --type-add 'js:*.{js,coffee}'
# Check for Rails UJS confirmation handling (fixed search)
rg "data-confirm|rails-ujs.*confirm" app/assets/javascripts/
Length of output: 6067
|
The message can be refined to be more urgent, but the pop up looks great and functional. |
|
The implementation looks great and effectively resolves the issue by introducing a clear confirmation prompt for late day submissions. The UI design aligns well with user experience best practices by providing clear messaging to users about the implications of their late submission, including the number of late days being used. The confirmation dialog is intuitive, with both "Cancel" and "OK" options prominently displayed. I tested the changes across multiple scenarios, including submissions within the grace period and significantly late submissions, and found the functionality to work as expected. The prompt consistently displayed accurate information and ensured users had the opportunity to confirm or cancel their actions. This update is ready to be merged. Great work! |
Added a Confirmation Prompt for Late Days
Description
Adjusted UI by adding a confirmation prompt for the use of late days that appears when pressing the late submit button.
Changed UI:
To implement the confirmation feature, I first identified the handin HTML erb form where users submit assignments and inspected the DOM to trace the submit button’s functionality. Debugging revealed that the button was a "fake" submit, controlled by a JavaScript event listener, preventing direct interaction with the controller. Through console logging in the Javascript file I found that I had no access to the database. I also did not have the ability to modify the controller’s redirect-heavy logic. I focused on the ERB file and discovered an embedded Ruby method that creates a confirmation popup before clicking the button would be processed. By leveraging existing code that calculated late day usage and using the embedded method, I implemented a solution that provided users with accurate feedback in a confirmation prompt.
Motivation and Context
Students mistakenly submit to the wrong assignments, either wasting late days unnecessarily or submitting to assignments where late days are not allowed. As a solution, we can add a confirmation prompt when students are about to use late days. Fixes #2211
How Has This Been Tested?
Tested locally to make sure that the confirmation prompt appears when submitting (see screenshots).
Types of changes
Checklist:
overcommit --install && overcommit --signto use pre-commit hook for linting