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.

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

General: Improve exercise view when using LTI #9329

Merged
merged 45 commits into from
Oct 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
376cc75
hiding navbar and footer if accessed via LTI
raffifasaro Sep 18, 2024
96cfb91
fix
raffifasaro Sep 18, 2024
6ca549e
fix
raffifasaro Sep 18, 2024
68e6f54
fix
raffifasaro Sep 18, 2024
2a5aa08
fix
raffifasaro Sep 19, 2024
8e48170
add lti service and try fix query param get
raffifasaro Sep 20, 2024
e24c825
Hide all but sidebar
raffifasaro Sep 20, 2024
0905544
fixes
raffifasaro Sep 20, 2024
47c051b
temp iFrame approach
raffifasaro Sep 21, 2024
bd156bf
Merge remote-tracking branch 'origin/develop' into chore/improve-lti-…
raffifasaro Sep 21, 2024
7361f42
set isLti over lti-launch
raffifasaro Sep 21, 2024
d6db31a
fix
raffifasaro Sep 21, 2024
25e6936
param test
raffifasaro Sep 21, 2024
638c369
param fix
raffifasaro Sep 21, 2024
7231939
hide sidebar
raffifasaro Sep 22, 2024
0b319d3
Merge remote-tracking branch 'origin/develop' into chore/improve-lti-…
raffifasaro Oct 1, 2024
c71659f
test
raffifasaro Oct 1, 2024
4069187
test
raffifasaro Oct 1, 2024
6cc2610
test
raffifasaro Oct 1, 2024
0fa20d0
test params
raffifasaro Oct 1, 2024
ffa2303
test without params
raffifasaro Oct 1, 2024
bb58b36
test print url
raffifasaro Oct 1, 2024
7c44790
test short url
raffifasaro Oct 1, 2024
34d1fe0
Merge remote-tracking branch 'origin/develop' into chore/improve-lti-…
raffifasaro Oct 2, 2024
5db7e5f
navigation fixes
raffifasaro Oct 2, 2024
6fe0fef
Formatting changes
raffifasaro Oct 2, 2024
e44e86f
Merge remote-tracking branch 'origin/develop' into chore/improve-lti-…
raffifasaro Oct 7, 2024
a82e340
merge fix
raffifasaro Oct 7, 2024
c8ca846
comment out test method
raffifasaro Oct 7, 2024
0696e8e
further formatting fixes
raffifasaro Oct 9, 2024
080370b
Merge branch 'develop' into chore/improve-lti-exercise-ui
raffifasaro Oct 10, 2024
4dde91c
Theme fix
raffifasaro Oct 10, 2024
427865d
Merge remote-tracking branch 'origin/chore/improve-lti-exercise-ui' i…
raffifasaro Oct 10, 2024
d185dbd
remove main component bottom padding
raffifasaro Oct 10, 2024
d2dcfaa
theme test
raffifasaro Oct 10, 2024
ae4ee18
ribbon removal for screenshots
raffifasaro Oct 10, 2024
f3e55cb
test fixes and banner readd
raffifasaro Oct 10, 2024
2dccb79
fixes
raffifasaro Oct 10, 2024
3991927
test client tests
raffifasaro Oct 10, 2024
32aa1b0
test
raffifasaro Oct 10, 2024
f9b95b6
test fixes
raffifasaro Oct 10, 2024
e18aa2a
remove test function
raffifasaro Oct 10, 2024
37141b2
remove commented out code
raffifasaro Oct 10, 2024
9df0b1a
add unsubscribers
raffifasaro Oct 10, 2024
e710bc2
add unsubscribers fix
raffifasaro Oct 10, 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
10 changes: 9 additions & 1 deletion src/main/webapp/app/lti/lti13-exercise-launch.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { HttpClient, HttpHeaders, HttpParams } from '@angular/common/http';
import { AccountService } from 'app/core/auth/account.service';
import { captureException } from '@sentry/angular';
import { SessionStorageService } from 'ngx-webstorage';
import { LtiService } from 'app/shared/service/lti.service';
import { Theme, ThemeService } from 'app/core/theme/theme.service';

type LtiLaunchResponse = {
targetLinkUri: string;
Expand All @@ -24,6 +26,8 @@ export class Lti13ExerciseLaunchComponent implements OnInit {
private accountService: AccountService,
private router: Router,
private sessionStorageService: SessionStorageService,
private ltiService: LtiService,
private themeService: ThemeService,
) {
this.isLaunching = true;
}
Expand Down Expand Up @@ -142,6 +146,10 @@ export class Lti13ExerciseLaunchComponent implements OnInit {
}

replaceWindowLocationWrapper(url: string): void {
window.location.replace(url);
this.ltiService.setLti(true);
this.themeService.applyThemeExplicitly(Theme.LIGHT);
const path = new URL(url).pathname;

this.router.navigate([path], { replaceUrl: true });
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<div class="d-flex justify-content-between horizontal-scroll">
@if (course) {
<div [ngClass]="{ 'sidebar-collapsed': isCollapsed }">
<div [ngClass]="{ 'sidebar-collapsed': isCollapsed }" *ngIf="!isLti">
krusche marked this conversation as resolved.
Show resolved Hide resolved
<jhi-sidebar [itemSelected]="exerciseSelected" [courseId]="courseId" [sidebarData]="sidebarData" [collapseState]="DEFAULT_COLLAPSE_STATE" [showFilter]="true" />
</div>

@if (exerciseSelected) {
<div class="vw-100 module-bg rounded-3">
<div class="vw-100 module-bg rounded-3" [ngStyle]="isLti ? { height: '90vh', position: 'relative' } : {}">
<router-outlet (deactivate)="onSubRouteDeactivate()" />
</div>
} @else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Exercise } from 'app/entities/exercise.model';
import { CourseStorageService } from 'app/course/manage/course-storage.service';
import { AccordionGroups, CollapseState, SidebarCardElement, SidebarData } from 'app/types/sidebar';
import { CourseOverviewService } from '../course-overview.service';
import { LtiService } from 'app/shared/service/lti.service';

const DEFAULT_UNIT_GROUPS: AccordionGroups = {
future: { entityData: [] },
Expand All @@ -34,6 +35,7 @@ const DEFAULT_COLLAPSE_STATE: CollapseState = {
export class CourseExercisesComponent implements OnInit, OnDestroy {
private parentParamSubscription: Subscription;
private courseUpdatesSubscription: Subscription;
private ltiSubscription: Subscription;

course?: Course;
courseId: number;
Expand All @@ -46,6 +48,7 @@ export class CourseExercisesComponent implements OnInit, OnDestroy {
sidebarExercises: SidebarCardElement[] = [];
isCollapsed: boolean = false;
readonly DEFAULT_COLLAPSE_STATE = DEFAULT_COLLAPSE_STATE;
isLti: boolean = false;
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved

constructor(
private courseStorageService: CourseStorageService,
Expand All @@ -54,6 +57,7 @@ export class CourseExercisesComponent implements OnInit, OnDestroy {
private programmingSubmissionService: ProgrammingSubmissionService,
private router: Router,
private courseOverviewService: CourseOverviewService,
private ltiService: LtiService,
) {}

ngOnInit() {
Expand All @@ -74,6 +78,10 @@ export class CourseExercisesComponent implements OnInit, OnDestroy {

this.exerciseForGuidedTour = this.guidedTourService.enableTourForCourseExerciseComponent(this.course, courseExerciseOverviewTour, true);

this.ltiSubscription = this.ltiService.isLti$.subscribe((isLti) => {
this.isLti = isLti;
});

// If no exercise is selected navigate to the lastSelected or upcoming exercise
this.navigateToExercise();
}
Expand Down Expand Up @@ -143,5 +151,6 @@ export class CourseExercisesComponent implements OnInit, OnDestroy {
ngOnDestroy(): void {
this.courseUpdatesSubscription?.unsubscribe();
this.parentParamSubscription?.unsubscribe();
this.ltiSubscription?.unsubscribe();
}
}
360 changes: 187 additions & 173 deletions src/main/webapp/app/overview/course-overview.component.html

Large diffs are not rendered by default.

14 changes: 12 additions & 2 deletions src/main/webapp/app/overview/course-overview.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import { ExamParticipationService } from 'app/exam/participate/exam-participatio
import { CourseConversationsComponent } from 'app/overview/course-conversations/course-conversations.component';
import { sortCourses } from 'app/shared/util/course.util';
import { CourseUnenrollmentModalComponent } from './course-unenrollment-modal.component';
import { LtiService } from 'app/shared/service/lti.service';

interface CourseActionItem {
title: string;
Expand Down Expand Up @@ -124,6 +125,8 @@ export class CourseOverviewComponent implements OnInit, OnDestroy, AfterViewInit
isExamStarted = false;
private examStartedSubscription: Subscription;
readonly MIN_DISPLAYED_COURSES: number = 6;
isLti: boolean = false;
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
private ltiSubscription: Subscription;
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved

// Properties to track hidden items for dropdown menu
dropdownOpen: boolean = false;
Expand Down Expand Up @@ -199,6 +202,7 @@ export class CourseOverviewComponent implements OnInit, OnDestroy, AfterViewInit
private profileService: ProfileService,
private modalService: NgbModal,
private examParticipationService: ExamParticipationService,
private ltiService: LtiService,
) {}

async ngOnInit() {
Expand Down Expand Up @@ -234,13 +238,18 @@ export class CourseOverviewComponent implements OnInit, OnDestroy, AfterViewInit
this.updateVisibleNavbarItems(window.innerHeight);
await this.updateRecentlyAccessedCourses();
this.isSidebarCollapsed = this.activatedComponentReference?.isCollapsed ?? false;
this.ltiSubscription = this.ltiService.isLti$.subscribe((isLti) => {
this.isLti = isLti;
});
}

/** Listen window resize event by height */
@HostListener('window: resize', ['$event'])
onResize() {
this.updateVisibleNavbarItems(window.innerHeight);
if (!this.anyItemHidden) this.itemsDrop.close();
if (this.itemsDrop) {
this.updateVisibleNavbarItems(window.innerHeight);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the updateNavbarItems functionality really strictly bound to the itemsDrop? I'm not 100% certain with that functionality, thus, the question.. :)

if (!this.anyItemHidden) this.itemsDrop.close();
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
}
}

/** Update sidebar item's hidden property based on the window height to display three-dots */
Expand Down Expand Up @@ -740,6 +749,7 @@ export class CourseOverviewComponent implements OnInit, OnDestroy, AfterViewInit
this.dashboardSubscription?.unsubscribe();
this.ngUnsubscribe.next();
this.ngUnsubscribe.complete();
this.ltiSubscription?.unsubscribe();
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
}

subscribeForQuizChanges() {
Expand Down
6 changes: 3 additions & 3 deletions src/main/webapp/app/shared/layouts/main/main.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
<div
id="page-wrapper"
class="page-wrapper"
[ngClass]="{ 'footer-height-dev': !isProduction || isTestServer }"
[ngClass]="{ 'footer-height-dev': !isProduction || isTestServer, 'no-padding': isLti }"
[ngStyle]="{ 'overflow-y': !isTestRunExam && isExamStarted ? 'hidden' : 'auto' }"
cdkScrollable
>
<jhi-page-ribbon />
@if (showSkeleton) {
@if (showSkeleton && !isLti) {
<div class="sticky-top">
<router-outlet name="navbar" />
</div>
Expand All @@ -23,7 +23,7 @@
<router-outlet />
}
<jhi-notification-popup />
@if (showSkeleton) {
@if (showSkeleton && !isLti) {
<div>
@if (isTestRunExam || !isExamStarted) {
<jhi-footer [ngClass]="{ 'footer-height-dev': !isProduction || isTestServer }" />
Expand Down
11 changes: 10 additions & 1 deletion src/main/webapp/app/shared/layouts/main/main.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { DOCUMENT } from '@angular/common';
import { AnalyticsService } from 'app/core/posthog/analytics.service';
import { Subscription } from 'rxjs';
import { ExamParticipationService } from 'app/exam/participate/exam-participation.service';
import { CourseManagementService } from '../../../course/manage/course-management.service';
import { CourseManagementService } from 'app/course/manage/course-management.service';
import { LtiService } from 'app/shared/service/lti.service';

@Component({
selector: 'jhi-main',
Expand All @@ -26,11 +27,13 @@ export class JhiMainComponent implements OnInit, OnDestroy {
examStartedSubscription: Subscription;
courseOverviewSubscription: Subscription;
testRunSubscription: Subscription;
ltiSubscription: Subscription;
isProduction: boolean = true;
isTestServer: boolean = false;
isExamStarted: boolean = false;
isTestRunExam: boolean = false;
isCourseOverview: boolean = false;
isLti: boolean = false;
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved

constructor(
private jhiLanguageHelper: JhiLanguageHelper,
Expand All @@ -44,6 +47,7 @@ export class JhiMainComponent implements OnInit, OnDestroy {
private document: Document,
private renderer: Renderer2,
private courseService: CourseManagementService,
private ltiService: LtiService,
) {
this.setupErrorHandling().then(undefined);
this.setupAnalytics().then(undefined);
Expand Down Expand Up @@ -120,6 +124,10 @@ export class JhiMainComponent implements OnInit, OnDestroy {
this.isCourseOverview = isPresent;
});

this.ltiSubscription = this.ltiService.isLti$.subscribe((isLti) => {
this.isLti = isLti;
});

this.themeService.initialize();
}

Expand All @@ -138,5 +146,6 @@ export class JhiMainComponent implements OnInit, OnDestroy {
this.examStartedSubscription?.unsubscribe();
this.testRunSubscription?.unsubscribe();
this.courseOverviewSubscription?.unsubscribe();
this.ltiSubscription?.unsubscribe();
}
}
15 changes: 15 additions & 0 deletions src/main/webapp/app/shared/service/lti.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { Injectable } from '@angular/core';
import { BehaviorSubject } from 'rxjs';

@Injectable({
providedIn: 'root',
})
export class LtiService {
constructor() {}
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
private ltiSubject = new BehaviorSubject<boolean>(false);
isLti$ = this.ltiSubject.asObservable();

setLti(isLti: boolean) {
this.ltiSubject.next(isLti);
}
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ describe('CourseOverviewComponent', () => {
fixture = TestBed.createComponent(CourseOverviewComponent);

component = fixture.componentInstance;
component.isLti = false;
courseService = TestBed.inject(CourseManagementService);
courseStorageService = TestBed.inject(CourseStorageService);
examParticipationService = TestBed.inject(ExamParticipationService);
Expand Down Expand Up @@ -646,14 +647,18 @@ describe('CourseOverviewComponent', () => {
});

it('should display content of dropdown when dropdownOpen changes', () => {
itemsDrop.open();
fixture.detectChanges();
expect(component.itemsDrop.isOpen).toBeTrue();
if (component.itemsDrop) {
itemsDrop.open();
fixture.detectChanges();
expect(component.itemsDrop.isOpen).toBeTrue();
}
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
});
it('should hide content of dropdown when dropdownOpen changes', () => {
itemsDrop.close();
fixture.detectChanges();
expect(component.itemsDrop.isOpen).toBeFalse();
if (component.itemsDrop) {
itemsDrop.close();
fixture.detectChanges();
expect(component.itemsDrop.isOpen).toBeFalse();
}
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
});

it('should display more icon and label if at least one item gets hidden in the sidebar', () => {
Expand All @@ -667,11 +672,13 @@ describe('CourseOverviewComponent', () => {
});

it('should change dropdownOpen when clicking on More', () => {
itemsDrop.close();
const clickOnMoreItem = fixture.nativeElement.querySelector('.three-dots');
clickOnMoreItem.click();
if (component.itemsDrop) {
itemsDrop.close();
const clickOnMoreItem = fixture.nativeElement.querySelector('.three-dots');
clickOnMoreItem.click();

expect(fixture.nativeElement.querySelector('.dropdown-content').hidden).toBeFalse();
expect(fixture.nativeElement.querySelector('.dropdown-content').hidden).toBeFalse();
}
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
raffifasaro marked this conversation as resolved.
Show resolved Hide resolved
});

it('should apply exam-wrapper and exam-is-active if exam is started', () => {
Expand Down
Loading