Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plagiarism checks: Enhance navigation to plagiarism cases from detection page #10078

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Plagiarism checks: Enhance navigation to plagiarism cases from detection page #10078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e67277d
add label translations and button
AjayvirS Dec 23, 2024
fa29224
add scrolling to exercise after navigation
AjayvirS Dec 23, 2024
0f78b42
fix optional operator
AjayvirS Dec 23, 2024
ced4c92
add tests
AjayvirS Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...p/course/plagiarism-cases/instructor-view/plagiarism-cases-instructor-view.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ <h4 jhiTranslate="artemisApp.plagiarism.cases.pageSubtitle"></h4>
</div>
</div>
@for (exercise of exercisesWithPlagiarismCases; track exercise; let i = $index) {
<div class="card mb-2">
<div #plagExerciseElement class="card mb-2" [id]="'exercise-with-plagiarism-case-' + exercise.id">
<div class="card-header">
<div class="row">
<div class="col-3">
Expand Down
25 changes: 23 additions & 2 deletions ...app/course/plagiarism-cases/instructor-view/plagiarism-cases-instructor-view.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { HttpResponse } from '@angular/common/http';
import { Component, OnInit } from '@angular/core';
import { Component, ElementRef, OnInit, effect, viewChildren } from '@angular/core';
import { ActivatedRoute } from '@angular/router';
import { PlagiarismCasesService } from 'app/course/plagiarism-cases/shared/plagiarism-cases.service';
import { PlagiarismCase } from 'app/exercises/shared/plagiarism/types/PlagiarismCase';
Expand All @@ -15,22 +15,32 @@ import { AlertService } from 'app/core/util/alert.service';
export class PlagiarismCasesInstructorViewComponent implements OnInit {
courseId: number;
examId?: number;
exerciseId?: number;
plagiarismCases: PlagiarismCase[] = [];
groupedPlagiarismCases: any; // maybe? { [key: number]: PlagiarismCase[] }
exercisesWithPlagiarismCases: Exercise[] = [];

exerciseWithPlagCasesElements = viewChildren<ElementRef>('plagExerciseElement');

readonly getIcon = getIcon;
readonly documentationType: DocumentationType = 'PlagiarismChecks';

constructor(
private plagiarismCasesService: PlagiarismCasesService,
private route: ActivatedRoute,
private alertService: AlertService,
) {}
) {
effect(() => {
if (this.exerciseId) {
this.scrollToExercise();
}
});
}

ngOnInit(): void {
this.courseId = Number(this.route.snapshot.paramMap.get('courseId'));
this.examId = Number(this.route.snapshot.paramMap.get('examId'));
this.exerciseId = Number(this.route.snapshot.queryParamMap?.get('exerciseId'));

const plagiarismCasesForInstructor$ = this.examId
? this.plagiarismCasesService.getExamPlagiarismCasesForInstructor(this.courseId, this.examId)
Expand Down Expand Up @@ -68,6 +78,17 @@ export class PlagiarismCasesInstructorViewComponent implements OnInit {
});
}

scrollToExercise() {
const element = this.exerciseWithPlagCasesElements().find((elem) => elem.nativeElement.id === 'exercise-with-plagiarism-case-' + this.exerciseId);
if (element) {
element.nativeElement.scrollIntoView({
behavior: 'smooth',
block: 'start',
inline: 'nearest',
});
}
}
Comment on lines +81 to +90
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the search results, I can now provide a final response as we've found evidence of fixed headers in the application:

Consider adding offset for fixed navbar when scrolling
The application has a fixed navbar with a height of 68px ($header-height: 68px) and uses sticky positioning. To ensure the target element is not hidden behind the navbar, add an offset:

  • Add a scroll margin to the target element:
element.nativeElement.style.scrollMarginTop = '70px';  // slightly more than header height
element.nativeElement.scrollIntoView({
    behavior: 'smooth',
    block: 'start',
    inline: 'nearest',
});
🔗 Analysis chain

Validate offset handling for scrollIntoView if headers or sticky toolbars exist.
If there is a top navbar or sticky element, consider adjusting the scroll position to ensure the target element is fully visible.

 element.nativeElement.scrollIntoView({
   behavior: 'smooth',
   block: 'start',
+  // potential offset logic if a fixed header obstructs the view
 });

Let me search for any fixed headers or navigation bars in the codebase that might affect the scrolling behavior.

Let me search specifically for the navbar component and its styling to verify if there are fixed headers that might affect scrolling.

Let me search for any global styles that might affect the navbar or header positioning.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found

Script:

#!/bin/bash
# Search for navigation components and fixed/sticky positioning
rg -l "position:\s*(fixed|sticky)" --type html
rg "navbar" --type html -l
rg "header" --type html -l

# Search for any CSS classes that might define fixed positioning
rg "\.fixed|\.sticky" --type css --type scss -l

# Look for navigation component definitions
ast-grep --pattern 'class $_ implements OnInit {
  $$$
  // Look for navbar/header related component
  $$$
}'

Length of output: 23385

Script:

#!/bin/bash
# Check navbar component implementation and its styling
cat src/main/webapp/app/shared/layouts/navbar/navbar.component.html
rg "position:\s*(fixed|sticky)" src/main/webapp/app/shared/layouts/navbar/navbar.component.html
rg "position:\s*(fixed|sticky)" src/main/webapp/content/scss/global.scss

Length of output: 20889

Script:

#!/bin/bash
# Search for navbar and header related styles in all scss files
fd -e scss -x rg "navbar|header" {} \;
fd -e scss -x rg "position:\s*(fixed|sticky)" {} \;

# Also check for any global styles that might affect scrolling
fd -e scss -x rg "scroll|overflow" {} \;

Length of output: 22543


/**
* calculate the total number of plagiarism cases
* @param plagiarismCases plagiarismCases in the course or exam
Expand Down
18 changes: 11 additions & 7 deletions ...webapp/app/exercises/shared/plagiarism/plagiarism-header/plagiarism-header.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ <h5 class="fw-medium">
</div>

<div class="plagiarism-header-right">
<button
[disabled]="comparison.status === plagiarismStatus.CONFIRMED || isLoading || exercise.teamMode"
class="btn btn-success btn-sm"
(click)="confirmPlagiarism()"
data-qa="confirm-plagiarism-button"
jhiTranslate="artemisApp.plagiarism.confirm"
></button>
@if (comparison.status !== plagiarismStatus.CONFIRMED && !isLoading && !exercise.teamMode) {
<button class="btn btn-success btn-sm" (click)="confirmPlagiarism()" data-qa="confirm-plagiarism-button" jhiTranslate="artemisApp.plagiarism.confirm"></button>
} @else {
<button
class="btn btn-primary btn-sm"
data-qa="view-plagiarism-cases-button"
jhiTranslate="artemisApp.plagiarism.viewCases"
[routerLink]="['/course-management', exercise.course?.id, 'plagiarism-cases']"
[queryParams]="{ exerciseId: exercise.id }"
></button>
}

<button
[disabled]="comparison.status === plagiarismStatus.DENIED || isLoading || exercise.teamMode"
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/de/plagiarism.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"plagiarism": {
"plagiarismDetection": "Plagiatskontrolle",
"confirm": "Bestätigen",
"viewCases": "Fälle ansehen",
"deny": "Ablehnen",
"denyAfterConfirmModalTitle": "Wechsel von Bestätigen zu Ablehnen",
"denyAfterConfirmModalText": "Bist du dir sicher, dass du die Entscheidung von \"Bestätigung des Plagiats\" in \"Ablehnung\" ändern möchtest? Dadurch wird der entsprechende Plagiatsfall einschließlich der Kommunikation mit dem/der Studierenden und des Urteils gelöscht und kann nicht rückgängig gemacht werden.",
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/en/plagiarism.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"plagiarism": {
"plagiarismDetection": "Plagiarism Detection",
"confirm": "Confirm",
"viewCases": "View Case(s)",
"deny": "Deny",
"denyAfterConfirmModalTitle": "Change from confirm to deny",
"denyAfterConfirmModalText": "Are you sure that you want to change the decision from confirming the plagiarism to denying it? This will delete the corresponding plagiarism case incl. the communication with the student and the verdict and cannot be undone.",
Expand Down
22 changes: 22 additions & 0 deletions ...t/javascript/spec/component/plagiarism/plagiarism-cases-instructor-view.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { DocumentationButtonComponent } from 'app/shared/components/documentatio
import { MockComponent } from 'ng-mocks';
import { NotificationService } from 'app/shared/notification/notification.service';
import { MockNotificationService } from '../../helpers/mocks/service/mock-notification.service';
import { ElementRef, signal } from '@angular/core';

jest.mock('app/shared/util/download.util', () => ({
downloadFile: jest.fn(),
Expand Down Expand Up @@ -198,4 +199,25 @@ describe('Plagiarism Cases Instructor View Component', () => {
expect(downloadSpy).toHaveBeenCalledOnce();
expect(downloadSpy).toHaveBeenCalledWith(new Blob(expectedBlob, { type: 'text/csv' }), 'plagiarism-cases.csv');
});

it('should scroll to the correct exercise element when scrollToExercise is called', () => {
component.exerciseId = 1;

const nativeElement1 = { id: 'exercise-with-plagiarism-case-1', scrollIntoView: jest.fn() };
const nativeElement2 = { id: 'exercise-with-plagiarism-case-2', scrollIntoView: jest.fn() };

const elementRef1 = new ElementRef(nativeElement1);
const elementRef2 = new ElementRef(nativeElement2);

component.exerciseWithPlagCasesElements = signal([elementRef1, elementRef2]);

component.scrollToExercise();

expect(nativeElement1.scrollIntoView).toHaveBeenCalledWith({
behavior: 'smooth',
block: 'start',
inline: 'nearest',
});
expect(nativeElement2.scrollIntoView).not.toHaveBeenCalled();
});
});
Loading