Modeling exercises: Fix Enter key navigation in exercise forms
#11490
Conversation
github-actions
bot
added
client
WalkthroughAdds Enter-key navigation handling to the modeling exercise update form via a hidden submit button and a new handler that advances focus while skipping textareas, contenteditable regions, Apollon editor container, and disabled/hidden fields. Also sets the modeling editor help button to type="button" to prevent accidental form submit. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Form as Update Form
participant Comp as ModelingExerciseUpdateComponent
participant DOM as Focusable Elements
User->>Form: Press Enter
Note over Form,Comp: Hidden submit button receives Enter
Form->>Comp: handleEnterKeyNavigation(event)
alt Focus in TEXTAREA / contentEditable / Apollon
Comp-->>Form: Prevent default, stop propagation, do nothing
else Focus in standard input/select/button
Comp->>DOM: Find next enabled, visible, focusable element
DOM-->>Comp: Next element or none
alt Next element found
Comp->>DOM: Focus next element (prevent form submit)
else No next element
Comp-->>Form: Allow default behavior or stay (no focus change)
end
end
sequenceDiagram
participant User
participant Editor as Modeling Editor Template
participant HelpBtn as Help Button (type="button")
User->>HelpBtn: Click / Enter
HelpBtn-->>Editor: Trigger help action only (no form submit)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
![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 (1)
src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts (1)
298-327: LGTM! Navigation logic correctly implements Enter-as-Tab behavior.The method properly:
- Prevents form submission via
preventDefault()andstopPropagation()- Preserves default Enter behavior in textareas and contentEditable regions
- Respects Apollon editor's internal input handling
- Advances focus to the next form control for standard inputs/selects
Optional: Consider a template literal for improved readability.
The focusable elements selector could use a template literal instead of string concatenation:
- const focusableElements = Array.from( - formRoot.querySelectorAll( - 'input:not([disabled]):not([readonly]):not([tabindex="-1"]):not([hidden]):not([type="hidden"]), ' + 'select:not([disabled]):not([tabindex="-1"]):not([hidden])', - ), - ) as HTMLElement[]; + const focusableElements = Array.from( + formRoot.querySelectorAll( + `input:not([disabled]):not([readonly]):not([tabindex="-1"]):not([hidden]):not([type="hidden"]), + select:not([disabled]):not([tabindex="-1"]):not([hidden])`, + ), + ) as HTMLElement[];As per coding guidelines: Single quotes are preferred for strings, but template literals improve readability for multi-line selectors.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.html(1 hunks)src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts(3 hunks)src/main/webapp/app/modeling/shared/modeling-editor/modeling-editor.component.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/**/*.html
⚙️ CodeRabbit configuration file
@if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
Files:
src/main/webapp/app/modeling/shared/modeling-editor/modeling-editor.component.htmlsrc/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.html
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: client-style
- GitHub Check: Build .war artifact
- GitHub Check: client-tests-selected
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (4)
src/main/webapp/app/modeling/shared/modeling-editor/modeling-editor.component.html (1)
72-72: LGTM! Correct fix to prevent unintended form submission.Adding
type="button"ensures this help button won't be triggered when Enter is pressed in form inputs. This directly addresses the root cause described in the PR objectives where the first<button>without an explicit type was being activated on Enter.src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts (2)
1-1: LGTM! Necessary import for form element access.The
ElementRefimport is correctly added to support the neweditFormElViewChild that enables keyboard navigation logic.
98-98: LGTM! ViewChild correctly configured for form element access.The ViewChild declaration properly uses
read: ElementRefto obtain a reference to the native form element, which is necessary for querying focusable elements within the form.src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.html (1)
2-2: LGTM! Hidden submit button correctly enables Enter-key navigation.The hidden submit button with
type="submit"ensures the browser triggers this button (and its click handler) when Enter is pressed in form inputs, rather than the first visible button. The implementation correctly integrates with thehandleEnterKeyNavigationmethod to provide Tab-like navigation behavior.The use of
style="display: none"is appropriate here as it's more robust than alternatives like[hidden]which can be overridden by CSS.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.spec.ts (1)
353-502: Solid test coverage for Enter key navigation, with minor improvements suggested.The new test suite comprehensively covers the key scenarios for
handleEnterKeyNavigation. The structure and mocking approach are sound.Consider these optional refinements:
- Protect DOM cleanup with try-finally blocks (lines 372-386, 388-403):
it('should not navigate when focused on TEXTAREA', () => { const textarea = document.createElement('textarea'); - document.body.appendChild(textarea); - textarea.focus(); - - const mockEvent = { - preventDefault: jest.fn(), - stopPropagation: jest.fn(), - } as any as Event; - - comp.handleEnterKeyNavigation(mockEvent); - - expect(mockEvent.preventDefault).toHaveBeenCalledOnce(); - document.body.removeChild(textarea); + try { + document.body.appendChild(textarea); + textarea.focus(); + + const mockEvent = { + preventDefault: jest.fn(), + stopPropagation: jest.fn(), + } as any as Event; + + comp.handleEnterKeyNavigation(mockEvent); + + expect(mockEvent.preventDefault).toHaveBeenCalledOnce(); + } finally { + document.body.removeChild(textarea); + } });Apply similar pattern to the contentEditable test (lines 388-403).
Strengthen assertions for skip scenarios (lines 372-445):
- Currently, tests only verify
preventDefaultwas called- Add assertions to confirm focus did not change, e.g.,
expect(document.activeElement).toBe(textarea);Verify focus remains on last element (lines 476-502):
- Add
expect(document.activeElement).toBe(input2);to confirm no unintended focus change occurred
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: Analyse
hakanduranC
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Closed by mistake |
github-project-automation
bot
moved this from Ready For Review
to Work In Progress
in Artemis Development
hakanduranC
changed the title
Modeling exercises: Fix Enter key navigation in exercise formsModeling exercises: Fix Enter key navigation in exercise forms
hakanduranC
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
2 participants
Checklist
General
Client
Motivation and Context
Fixes #9641 - Issue where pressing Enter key in form input fields or Apollon Editor unexpectedly opens the help modal instead of providing normal field navigation behavior.
Root Cause: HTML forms without explicit submit buttons automatically trigger the first
<button>element when Enter is pressed. The modeling exercise form's first button was the help button, causing unwanted modal activation.Description
Implemented hidden submit button solution that intercepts Enter key events and provides appropriate navigation behavior:
Technical Implementation:
modeling-exercise-update.component.htmlhandleEnterKeyNavigation()method with smart component boundary detectionFiles Changed:
modeling-exercise-update.component.html: Added hidden submit buttonmodeling-exercise-update.component.ts: Added Enter key navigation logicSteps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Screenshots
2025-08-27.10-55-07.mp4
Summary by CodeRabbit
New Features
Bug Fixes
Tests