Assessment: Scroll down to complaint form after clicking on a complain button
#10352
Conversation
…github.com/badkeyy/Artemis into bugfix/scroll-down-after-clicking-complain
github-actions
bot
added
tests
client
WalkthroughThis pull request updates the complaints student view component by modifying both its HTML template and TypeScript logic. The HTML now conditionally renders buttons for filing a complaint or requesting more feedback based on user authorization and the absence of an existing complaint. The click event handlers have been updated to call a new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as View (HTML)
participant C as ComplaintsStudentViewComponent
participant D as DOM (Scrollpoint)
U->>V: Click "File Complaint" / "Request More Feedback"
V->>C: Call openComplaintForm(ComplaintType)
C->>C: Set complaint type and trigger ChangeDetectorRef update
C->>C: Invoke scrollToComplaint()
C->>D: Scroll to complaintScrollpoint element
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
Documentation and Community
|
![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 (2)
src/main/webapp/app/complaints/complaints-for-students/complaints-student-view.component.ts (1)
173-175: Consider enhancing scroll behavior robustness.While the current implementation works, it could be improved for better maintainability and error handling.
Consider this refactor:
- private scrollToComplaint(): void { - this.complaintScrollpoint?.nativeElement.scrollIntoView({ behavior: 'smooth', block: 'end' }); - } + private readonly SCROLL_OPTIONS = { behavior: 'smooth' as const, block: 'end' as const }; + + private scrollToComplaint(): void { + try { + this.complaintScrollpoint?.nativeElement.scrollIntoView(this.SCROLL_OPTIONS); + } catch (error) { + console.warn('Failed to scroll to complaint form:', error); + } + }src/test/javascript/spec/component/complaints/complaint-student-view.component.spec.ts (1)
242-305: Consider reducing test code duplication.While the tests are thorough, there's duplicate code for scroll mock setup that could be extracted.
Consider creating a helper function:
+ function setupScrollMock(component: ComplaintsStudentViewComponent): jest.Mock { + const scrollIntoViewMock = jest.fn(); + component.complaintScrollpoint = { + nativeElement: { + scrollIntoView: scrollIntoViewMock, + }, + } as ElementRef; + return scrollIntoViewMock; + } it('should set complaint type COMPLAINT and scroll to complaint form when pressing complaint', fakeAsync(() => { testInitWithResultStub(of()); // ... other setup ... - const scrollIntoViewMock = jest.fn(); - component.complaintScrollpoint = { - nativeElement: { - scrollIntoView: scrollIntoViewMock, - }, - } as ElementRef; + const scrollIntoViewMock = setupScrollMock(component); // ... rest of the test }));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/webapp/app/complaints/complaints-for-students/complaints-student-view.component.html(3 hunks)src/main/webapp/app/complaints/complaints-for-students/complaints-student-view.component.ts(4 hunks)src/test/javascript/spec/component/complaints/complaint-student-view.component.spec.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`src/test/javascript/spec/**/*.ts`: jest: true; mock: NgMock...
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/complaints/complaint-student-view.component.spec.ts
`src/main/webapp/**/*.html`: @if and @for are new and valid ...
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/complaints/complaints-for-students/complaints-student-view.component.html
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/...
src/main/webapp/app/complaints/complaints-for-students/complaints-student-view.component.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-tests
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (6)
src/main/webapp/app/complaints/complaints-for-students/complaints-student-view.component.ts (2)
1-44: LGTM! Clean imports and proper component setup.The imports are well-organized, and the ViewChild decorator is correctly used for the scroll functionality.
164-168: LGTM! Well-structured complaint form handling.The method properly handles state updates and ensures the view is ready before scrolling.
src/main/webapp/app/complaints/complaints-for-students/complaints-student-view.component.html (2)
27-27: LGTM! Clean event handling implementation.The click handlers are properly updated to use the new openComplaintForm method with correct ComplaintType values.
Also applies to: 42-42
63-63: LGTM! Well-placed scroll target element.The scroll point div is correctly positioned after the complaint form for proper scrolling behavior.
src/test/javascript/spec/component/complaints/complaint-student-view.component.spec.ts (2)
28-29: LGTM! Proper test imports.The necessary imports for ElementRef and ComplaintType are correctly added.
151-182: LGTM! Comprehensive exam mode scroll testing.The test properly verifies both the complaint type setting and scroll behavior, with good use of fakeAsync and mocking.
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
src/main/webapp/app/complaints/complaints-for-students/complaints-student-view.component.ts
Show resolved
Hide resolved
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
|
Tested on TS5. No Problems noticed :) |
krusche
changed the title
Grading: Scroll down to complaint form after clicking on a complain buttonAssessment: Scroll down to complaint form after clicking on a complain button
krusche
deleted the
bugfix/grading/scroll-down-after-clicking-complain
branch
Checklist
General
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
Fixes #9735
Description
Sometimes students were not able to see that the complaint form opened after pressing the "Complain" button. With this PR the browser windows gets scrolled down to the end of the form (To after the submit button) after pressing the button.
Steps for Testing
Prerequisites:
Exam Mode 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
Test Coverage
Client
Screenshots
Summary by CodeRabbit
New Features
Tests