Modeling exercises: Enhance modeling feedback layout
#10238
Conversation
…unreferenced feedback filtering
…ub.com/ls1intum/Artemis into bugfix/unreferenced-feedback-filtering
github-actions
bot
added
tests
client
WalkthroughThe pull request introduces modifications to the modeling submission component across HTML, SCSS, and TypeScript files. Key updates include the addition of new input properties to components like Changes
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/exercises/modeling/participate/modeling-submission.component.scss (1)
17-20: Consider adding responsive breakpoints for better mobile support.The fixed
max-width: 70vwmight cause content overflow on smaller screens. Consider adding media queries to adjust the width based on screen size..editor-assessment { max-width: 70vw; max-height: 100vh; + @media (max-width: 768px) { + max-width: 90vw; + } + @media (max-width: 480px) { + max-width: 100vw; + } }src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html (1)
Line range hint
1-204: Consider standardizing string literal quotes.For better consistency, consider standardizing the use of quotes throughout the template. Currently, there's a mix of single quotes (e.g., 'col-12 editor-large') and double quotes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html(3 hunks)src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.scss(1 hunks)src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.ts(3 hunks)src/main/webapp/app/exercises/shared/result/result.utils.ts(1 hunks)src/test/javascript/spec/component/utils/result.utils.spec.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/test/javascript/spec/component/utils/result.utils.spec.ts (1)
Pattern 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/main/webapp/app/exercises/modeling/participate/modeling-submission.component.ts (1)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html (1)
Pattern 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/exercises/shared/result/result.utils.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (7)
src/test/javascript/spec/component/utils/result.utils.spec.ts (1)
4-4: LGTM! Test coverage for the new feedback filtering function looks good.The test case properly verifies that
getAutomaticUnreferencedFeedbackcorrectly filters out non-automatic feedback and referenced feedback.Also applies to: 34-43
src/main/webapp/app/exercises/shared/result/result.utils.ts (1)
128-134: LGTM! Clean implementation of the automatic feedback filter.The function correctly filters for unreferenced feedback of type
AUTOMATICwith clear documentation.src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.ts (1)
18-18: LGTM! Consistent update to use the new feedback filtering function.The getter method correctly uses
getAutomaticUnreferencedFeedbackwhile maintaining the existing assessment result checks.Also applies to: 699-699
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html (4)
58-58: LGTM! Enhanced result history visibility.The addition of
[selectedResultId]binding improves the user experience by highlighting the current selection in the submission history, which aligns with the PR objectives.
64-64: LGTM! Improved layout control.The addition of
[collapsed]binding implements the requirement to collapse the problem statement by default in feedback view, optimizing screen space for editor and assessment areas.
Line range hint
68-188: LGTM! Enhanced layout structure and responsiveness.The new container structure implements several key improvements:
- Responsive layout using flex classes
- Proper overflow handling for long feedback lists
- Clear separation of editor and assessment columns
These changes align with the PR objectives for improving screen utilization and preventing layout issues.
199-204: LGTM! Updated Angular syntax.The conditional rendering blocks correctly use the new
@ifsyntax as per coding guidelines, replacing the older*ngIfsyntax.
LeonWehrhahn
changed the title
Modeling: Enhance Modeling Feedback Layout
LeonWehrhahn
changed the title
Modeling: Enhance Modeling Feedback LayoutModeling: Enhance Modeling Feedback Layout
LeonWehrhahn
changed the title
Modeling: Enhance Modeling Feedback LayoutModeling exercises : Enhance Modeling Feedback Layout
LeonWehrhahn
changed the title
Modeling exercises : Enhance Modeling Feedback LayoutModeling exercises: Enhance Modeling Feedback Layout
LeonWehrhahn
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
LeonWehrhahn
added
deploy:artemis-test1
and removed
deployment-error
|
github-actions
bot
added
the
deployment-error
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.ts (2)
246-270: Optimize submission history sorting logic.The current implementation can be improved for better maintainability and code reuse.
Consider applying these improvements:
private fetchSubmissionHistory() { return this.modelingSubmissionService.getSubmissionsWithResultsForParticipation(this.participationId!).pipe( catchError((error: HttpErrorResponse) => { onError(this.alertService, error); return of([]); }), tap((submissions: ModelingSubmission[]) => { this.sortedSubmissionHistory = submissions.sort((a, b) => { - const latestResultA = this.sortResultsByCompletionDate(a.results ?? [])[0]; - const latestResultB = this.sortResultsByCompletionDate(b.results ?? [])[0]; - const dateA = latestResultA?.completionDate ? dayjs(latestResultA.completionDate).valueOf() : 0; - const dateB = latestResultB?.completionDate ? dayjs(latestResultB.completionDate).valueOf() : 0; - return dateA - dateB; + const getLatestCompletionDate = (submission: ModelingSubmission) => { + const latestResult = this.sortResultsByCompletionDate(submission.results ?? [])[0]; + return latestResult?.completionDate ? dayjs(latestResult.completionDate).valueOf() : 0; + }; + return getLatestCompletionDate(a) - getLatestCompletionDate(b); }); this.sortedResultHistory = this.sortedSubmissionHistory .map((submission) => { - let latestResult: Result | undefined; - if (submission?.results && submission.results.length > 0) { - const sortedResults = [...submission.results].sort((a, b) => { - const dateA = a.completionDate ? dayjs(a.completionDate).valueOf() : 0; - const dateB = b.completionDate ? dayjs(b.completionDate).valueOf() : 0; - return dateB - dateA; - }); - latestResult = sortedResults[0]; - } + const latestResult = submission?.results?.length ? + this.sortResultsByCompletionDate(submission.results)[0] : undefined; if (latestResult) { latestResult.participation = submission.participation; } return latestResult; }) .filter((result): result is Result => !!result); }), ); }
426-427: Simplify condition using optional chaining.The condition check can be made more concise using optional chaining.
Apply this improvement:
- const latestResult = getLatestSubmissionResult(this.submission); - if (latestResult && latestResult.completionDate && (this.isAfterAssessmentDueDate || latestResult.assessmentType === AssessmentType.AUTOMATIC_ATHENA)) { + if (getLatestSubmissionResult(this.submission)?.completionDate && + (this.isAfterAssessmentDueDate || getLatestSubmissionResult(this.submission)?.assessmentType === AssessmentType.AUTOMATIC_ATHENA)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 427-427: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.scss(1 hunks)src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.scss
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.ts (1)
🪛 Biome (1.9.4)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.ts
[error] 427-427: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: server-tests-mysql
🔇 Additional comments (2)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.ts (2)
59-59: LGTM!The import statement is correctly placed with other component imports.
277-279: LGTM!The date comparison logic using dayjs is correct and follows best practices.
LeonWehrhahn
added
deploy:artemis-test3
and removed
deployment-error
LeonWehrhahn
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
GitHub Actions
Inactive
maximiliansoelch
added
ready to merge
maintainer-approved
krusche
changed the title
Modeling exercises: Enhance Modeling Feedback LayoutModeling exercises: Enhance modeling feedback layout
Checklist
General
Client
Motivation and Context
This PR streamlines the modeling feedback view for better usability. Previously, long feedback could overlap the editor, and the submission history lacked a clear indication of the current selection. Additionally, the submission history items were not sorted correctly. This revision resolves feedback overlap, visually highlights the active submission in the history, corrects the submission history sorting, and removes the displayed complaint inputs for AI feedback.
Description
selectedResultIdtojhi-result-history, to highlight the current selected submission.modeling-submission.component.tsto correctly order submission history items by submission time.Steps for Testing
Prerequisites:
jhi-resizeable-containeris collapsed by default.overflow-autoand doesn't break the layout.Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Performance Tests
Screenshots
Submission History Highlighting: This screenshot demonstrates the visual highlighting of the currently selected submission within the result history timeline.
Responsive Layout for Wide Models: This screenshot showcases the improved layout when viewing ultra-wide UML models. The editor and assessment areas are now proportionally adjusted, ensuring the model and feedback are viewable at the same time.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style
.editor-assessmentCSS class for better layout control.Bug Fixes
The changes improve the user interface and feedback display for modeling exercise submissions.