From 0c998b8f8b83861d52e238d71ea8bb12d15560e7 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Tue, 14 Mar 2023 13:52:23 +0100 Subject: [PATCH 1/2] MOBILE-4193 core: Consolidate module icons sources --- .../services/recentlyaccesseditems.ts | 7 +- src/addons/block/timeline/classes/section.ts | 82 ++++++++++++------- .../timeline/components/timeline/timeline.ts | 54 +++++++----- .../calendar/components/calendar/calendar.ts | 4 +- .../upcoming-events/upcoming-events.ts | 2 +- src/addons/calendar/pages/day/day.page.ts | 2 +- src/addons/calendar/pages/event/event.page.ts | 2 +- .../calendar/services/calendar-helper.ts | 10 ++- src/addons/calendar/services/calendar.ts | 1 + .../mod/label/services/handlers/module.ts | 9 +- .../mod/lti/services/handlers/module.ts | 11 ++- .../course/classes/module-base-handler.ts | 13 ++- .../course/services/module-delegate.ts | 7 +- .../grades/pages/course/course.page.ts | 2 +- .../features/grades/services/grades-helper.ts | 22 +++-- 15 files changed, 147 insertions(+), 81 deletions(-) diff --git a/src/addons/block/recentlyaccesseditems/services/recentlyaccesseditems.ts b/src/addons/block/recentlyaccesseditems/services/recentlyaccesseditems.ts index cee58917f..cbf2c5187 100644 --- a/src/addons/block/recentlyaccesseditems/services/recentlyaccesseditems.ts +++ b/src/addons/block/recentlyaccesseditems/services/recentlyaccesseditems.ts @@ -18,6 +18,7 @@ import { CoreDomUtils } from '@services/utils/dom'; import { CoreCourse } from '@features/course/services/course'; import { CoreSiteWSPreSets } from '@classes/site'; import { makeSingleton } from '@singletons'; +import { CoreCourseModuleDelegate } from '@features/course/services/module-delegate'; const ROOT_CACHE_KEY = 'AddonBlockRecentlyAccessedItems:'; @@ -54,15 +55,15 @@ export class AddonBlockRecentlyAccessedItemsProvider { const cmIds: number[] = []; - items = items.map((item) => { + items = await Promise.all(items.map(async (item) => { const modicon = item.icon && CoreDomUtils.getHTMLElementAttribute(item.icon, 'src'); - item.iconUrl = CoreCourse.getModuleIconSrc(item.modname, modicon || undefined); + item.iconUrl = await CoreCourseModuleDelegate.getModuleIconSrc(item.modname, modicon || undefined); item.iconTitle = item.icon && CoreDomUtils.getHTMLElementAttribute(item.icon, 'title'); cmIds.push(item.cmid); return item; - }); + })); // Check if the viewed module should be updated for each activity. const lastViewedMap = await CoreCourse.getCertainModulesViewed(cmIds, site.getId()); diff --git a/src/addons/block/timeline/classes/section.ts b/src/addons/block/timeline/classes/section.ts index 72301c42c..ae82df5a0 100644 --- a/src/addons/block/timeline/classes/section.ts +++ b/src/addons/block/timeline/classes/section.ts @@ -15,9 +15,10 @@ import { AddonBlockTimeline } from '@addons/block/timeline/services/timeline'; import { AddonCalendarEvent } from '@addons/calendar/services/calendar'; import { CoreCourse } from '@features/course/services/course'; +import { CoreCourseModuleDelegate } from '@features/course/services/module-delegate'; import { CoreEnrolledCourseDataWithOptions } from '@features/courses/services/courses-helper'; import { CoreTimeUtils } from '@services/utils/time'; -import { BehaviorSubject } from 'rxjs'; +import { BehaviorSubject, Observable } from 'rxjs'; /** * A collection of events displayed in the timeline block. @@ -28,12 +29,8 @@ export class AddonBlockTimelineSection { overdue: boolean; dateRange: AddonBlockTimelineDateRange; course?: CoreEnrolledCourseDataWithOptions; - data$: BehaviorSubject<{ - events: AddonBlockTimelineDayEvents[]; - lastEventId?: number; - canLoadMore: boolean; - loadingMore: boolean; - }>; + + private dataSubject$: BehaviorSubject; constructor( search: string | null, @@ -47,30 +44,42 @@ export class AddonBlockTimelineSection { this.overdue = overdue; this.dateRange = dateRange; this.course = course; - this.data$ = new BehaviorSubject({ - events: courseEvents ? this.reduceEvents(courseEvents, overdue, dateRange) : [], + this.dataSubject$ = new BehaviorSubject({ + events: [], lastEventId: canLoadMore, canLoadMore: typeof canLoadMore !== 'undefined', loadingMore: false, }); + + if (courseEvents) { + // eslint-disable-next-line promise/catch-or-return + this.reduceEvents(courseEvents, overdue, dateRange).then(events => this.dataSubject$.next({ + ...this.dataSubject$.value, + events, + })); + } + } + + get data$(): Observable { + return this.dataSubject$; } /** * Load more events. */ async loadMore(): Promise { - this.data$.next({ - ...this.data$.value, + this.dataSubject$.next({ + ...this.dataSubject$.value, loadingMore: true, }); - const lastEventId = this.data$.value.lastEventId; + const lastEventId = this.dataSubject$.value.lastEventId; const { events, canLoadMore } = this.course ? await AddonBlockTimeline.getActionEventsByCourse(this.course.id, lastEventId, this.search ?? '') : await AddonBlockTimeline.getActionEventsByTimesort(lastEventId, this.search ?? ''); - this.data$.next({ - events: this.data$.value.events.concat(this.reduceEvents(events, this.overdue, this.dateRange)), + this.dataSubject$.next({ + events: this.dataSubject$.value.events.concat(await this.reduceEvents(events, this.overdue, this.dateRange)), lastEventId: canLoadMore, canLoadMore: canLoadMore !== undefined, loadingMore: false, @@ -85,32 +94,35 @@ export class AddonBlockTimelineSection { * @param dateRange Date range to filter events. * @returns Day events list. */ - private reduceEvents( + private async reduceEvents( events: AddonCalendarEvent[], overdue: boolean, { from, to }: AddonBlockTimelineDateRange, - ): AddonBlockTimelineDayEvents[] { + ): Promise { const filterDates: AddonBlockTimelineFilterDates = { now: CoreTimeUtils.timestamp(), midnight: AddonBlockTimeline.getDayStart(), start: AddonBlockTimeline.getDayStart(from), end: typeof to === 'number' ? AddonBlockTimeline.getDayStart(to) : undefined, }; - const eventsByDates = events - .filter((event) => this.filterEvent(event, overdue, filterDates)) - .map((event) => this.mapToTimelineEvent(event, filterDates.now)) - .reduce((filteredEvents, event) => { - const dayTimestamp = CoreTimeUtils.getMidnightForTimestamp(event.timesort); + const timelineEvents = await Promise.all( + events + .filter((event) => this.filterEvent(event, overdue, filterDates)) + .map((event) => this.mapToTimelineEvent(event, filterDates.now)), + ); - filteredEvents[dayTimestamp] = filteredEvents[dayTimestamp] ?? { - dayTimestamp, - events: [], - } as AddonBlockTimelineDayEvents; + const eventsByDates = timelineEvents.reduce((filteredEvents, event) => { + const dayTimestamp = CoreTimeUtils.getMidnightForTimestamp(event.timesort); - filteredEvents[dayTimestamp].events.push(event); + filteredEvents[dayTimestamp] = filteredEvents[dayTimestamp] ?? { + dayTimestamp, + events: [], + } as AddonBlockTimelineDayEvents; - return filteredEvents; - }, {} as Record); + filteredEvents[dayTimestamp].events.push(event); + + return filteredEvents; + }, {} as Record); return Object.values(eventsByDates); } @@ -151,20 +163,30 @@ export class AddonBlockTimelineSection { * @param now Current time. * @returns Timeline event. */ - private mapToTimelineEvent(event: AddonCalendarEvent, now: number): AddonBlockTimelineEvent { + private async mapToTimelineEvent(event: AddonCalendarEvent, now: number): Promise { const modulename = event.modulename || event.icon.component; return { ...event, modulename, overdue: event.timesort < now, - iconUrl: CoreCourse.getModuleIconSrc(event.icon.component), + iconUrl: await CoreCourseModuleDelegate.getModuleIconSrc(event.icon.component, event.icon.iconurl), iconTitle: CoreCourse.translateModuleName(modulename), } as AddonBlockTimelineEvent; } } +/** + * Section data. + */ +export type AddonBlockTimelineSectionData = { + events: AddonBlockTimelineDayEvents[]; + lastEventId?: number; + canLoadMore: boolean; + loadingMore: boolean; +}; + /** * Timestamps to use during event filtering. */ diff --git a/src/addons/block/timeline/components/timeline/timeline.ts b/src/addons/block/timeline/components/timeline/timeline.ts index 70bbad552..85e071d7a 100644 --- a/src/addons/block/timeline/components/timeline/timeline.ts +++ b/src/addons/block/timeline/components/timeline/timeline.ts @@ -22,7 +22,7 @@ import { CoreCoursesHelper, CoreEnrolledCourseDataWithOptions } from '@features/ import { CoreCourses } from '@features/courses/services/courses'; import { CoreCourseOptionsDelegate } from '@features/course/services/course-options-delegate'; import { BehaviorSubject, combineLatest, Observable, of, Subject } from 'rxjs'; -import { catchError, distinctUntilChanged, map, share, tap } from 'rxjs/operators'; +import { catchError, distinctUntilChanged, map, share, tap, mergeAll } from 'rxjs/operators'; import { AddonBlockTimelineDateRange, AddonBlockTimelineSection } from '@addons/block/timeline/classes/section'; import { FormControl } from '@angular/forms'; import { formControlValue, resolved } from '@/core/utils/rxjs'; @@ -198,6 +198,7 @@ export class AddonBlockTimelineComponent implements OnInit, ICoreBlockComponent } }), resolved(), + mergeAll(), catchError(error => { // An error ocurred in the function, log the error and just resolve the observable so the workflow continues. this.logger.error(error); @@ -205,7 +206,7 @@ export class AddonBlockTimelineComponent implements OnInit, ICoreBlockComponent // Error getting data, fail. CoreDomUtils.showErrorModalDefault(error, this.fetchContentDefaultError, true); - return of([]); + return of([] as AddonBlockTimelineSection[]); }), share(), tap(() => (this.loaded = true)), @@ -224,12 +225,12 @@ export class AddonBlockTimelineComponent implements OnInit, ICoreBlockComponent search: string | null, overdue: boolean, dateRange: AddonBlockTimelineDateRange, - ): Promise { + ): Promise> { const section = new AddonBlockTimelineSection(search, overdue, dateRange); await section.loadMore(); - return section.data$.value.events.length > 0 ? [section] : []; + return section.data$.pipe(map(({ events }) => events.length > 0 ? [section] : [])); } /** @@ -246,29 +247,38 @@ export class AddonBlockTimelineComponent implements OnInit, ICoreBlockComponent overdue: boolean, dateRange: AddonBlockTimelineDateRange, courses: CoreEnrolledCourseDataWithOptions[], - ): Promise { + ): Promise> { // Do not filter courses by date because they can contain activities due. const courseIds = courses.map(course => course.id); const gracePeriod = await this.getCoursesGracePeriod(); const courseEvents = await AddonBlockTimeline.getActionEventsByCourses(courseIds, search ?? ''); - return courses - .filter( - course => - !course.hidden && - !CoreCoursesHelper.isPastCourse(course, gracePeriod.after) && - !CoreCoursesHelper.isFutureCourse(course, gracePeriod.after, gracePeriod.before) && - courseEvents[course.id].events.length > 0, - ) - .map(course => new AddonBlockTimelineSection( - search, - overdue, - dateRange, - course, - courseEvents[course.id].events, - courseEvents[course.id].canLoadMore, - )) - .filter(section => section.data$.value.events.length > 0); + return combineLatest( + courses + .filter( + course => + !course.hidden && + !CoreCoursesHelper.isPastCourse(course, gracePeriod.after) && + !CoreCoursesHelper.isFutureCourse(course, gracePeriod.after, gracePeriod.before) && + courseEvents[course.id].events.length > 0, + ) + .map(course => { + const section = new AddonBlockTimelineSection( + search, + overdue, + dateRange, + course, + courseEvents[course.id].events, + courseEvents[course.id].canLoadMore, + ); + + return section.data$.pipe(map(({ events }) => events.length > 0 ? section : null)); + }), + ).pipe( + map(sections => sections.filter( + (section: AddonBlockTimelineSection | null): section is AddonBlockTimelineSection => !!section, + )), + ); } /** diff --git a/src/addons/calendar/components/calendar/calendar.ts b/src/addons/calendar/components/calendar/calendar.ts index 77c077fb9..0295e9d15 100644 --- a/src/addons/calendar/components/calendar/calendar.ts +++ b/src/addons/calendar/components/calendar/calendar.ts @@ -551,7 +551,9 @@ class AddonCalendarMonthSlidesItemsManagerSource extends CoreSwipeSlidesDynamicI day.eventsFormated = day.eventsFormated || []; day.filteredEvents = day.filteredEvents || []; // Format online events. - const onlineEventsFormatted = day.events.map((event) => AddonCalendarHelper.formatEventData(event)); + const onlineEventsFormatted = await Promise.all( + day.events.map((event) => AddonCalendarHelper.formatEventData(event)), + ); day.eventsFormated = day.eventsFormated.concat(onlineEventsFormatted); diff --git a/src/addons/calendar/components/upcoming-events/upcoming-events.ts b/src/addons/calendar/components/upcoming-events/upcoming-events.ts index 0a0205a06..4586fd259 100644 --- a/src/addons/calendar/components/upcoming-events/upcoming-events.ts +++ b/src/addons/calendar/components/upcoming-events/upcoming-events.ts @@ -165,7 +165,7 @@ export class AddonCalendarUpcomingEventsComponent implements OnInit, DoCheck, On async fetchEvents(): Promise { // Don't pass courseId and categoryId, we'll filter them locally. const result = await AddonCalendar.getUpcomingEvents(); - this.onlineEvents = result.events.map((event) => AddonCalendarHelper.formatEventData(event)); + this.onlineEvents = await Promise.all(result.events.map((event) => AddonCalendarHelper.formatEventData(event))); // Merge the online events with offline data. this.events = this.mergeEvents(); // Filter events by course. diff --git a/src/addons/calendar/pages/day/day.page.ts b/src/addons/calendar/pages/day/day.page.ts index 793de1e54..65af26e37 100644 --- a/src/addons/calendar/pages/day/day.page.ts +++ b/src/addons/calendar/pages/day/day.page.ts @@ -672,7 +672,7 @@ class AddonCalendarDaySlidesItemsManagerSource extends CoreSwipeSlidesDynamicIte try { // Don't pass courseId and categoryId, we'll filter them locally. result = await AddonCalendar.getDayEvents(day.moment.year(), day.moment.month() + 1, day.moment.date()); - preloadedDay.onlineEvents = result.events.map((event) => AddonCalendarHelper.formatEventData(event)); + preloadedDay.onlineEvents = await Promise.all(result.events.map((event) => AddonCalendarHelper.formatEventData(event))); } catch (error) { // Allow navigating to non-cached days in offline (behave as if using emergency cache). if (CoreNetwork.isOnline()) { diff --git a/src/addons/calendar/pages/event/event.page.ts b/src/addons/calendar/pages/event/event.page.ts index a94f5d43f..ffe9f4092 100644 --- a/src/addons/calendar/pages/event/event.page.ts +++ b/src/addons/calendar/pages/event/event.page.ts @@ -197,7 +197,7 @@ export class AddonCalendarEventPage implements OnInit, OnDestroy { // Get the event data. if (this.eventId >= 0) { const event = await AddonCalendar.getEventById(this.eventId); - this.event = AddonCalendarHelper.formatEventData(event); + this.event = await AddonCalendarHelper.formatEventData(event); } try { diff --git a/src/addons/calendar/services/calendar-helper.ts b/src/addons/calendar/services/calendar-helper.ts index 024d099aa..ca154b7ee 100644 --- a/src/addons/calendar/services/calendar-helper.ts +++ b/src/addons/calendar/services/calendar-helper.ts @@ -37,6 +37,7 @@ import { AddonCalendarOfflineEventDBRecord } from './database/calendar-offline'; import { CoreCategoryData } from '@features/courses/services/courses'; import { CoreTimeUtils } from '@services/utils/time'; import { CoreReminders, CoreRemindersService } from '@features/reminders/services/reminders'; +import { CoreCourseModuleDelegate } from '@features/course/services/module-delegate'; /** * Context levels enumeration. @@ -164,9 +165,9 @@ export class AddonCalendarHelperProvider { * @param event Event to format. * @returns The formatted event to display. */ - formatEventData( + async formatEventData( event: AddonCalendarEvent | AddonCalendarEventBase | AddonCalendarGetEventsEvent, - ): AddonCalendarEventToDisplay { + ): Promise { const eventFormatted: AddonCalendarEventToDisplay = { ...event, @@ -182,7 +183,10 @@ export class AddonCalendarHelperProvider { }; if (event.modulename) { - eventFormatted.eventIcon = CoreCourse.getModuleIconSrc(event.modulename); + eventFormatted.eventIcon = await CoreCourseModuleDelegate.getModuleIconSrc( + event.modulename, + 'icon' in event ? event.icon.iconurl : undefined, + ); eventFormatted.moduleIcon = eventFormatted.eventIcon; eventFormatted.iconTitle = CoreCourse.translateModuleName(event.modulename); } diff --git a/src/addons/calendar/services/calendar.ts b/src/addons/calendar/services/calendar.ts index ce5142338..2fa158b92 100644 --- a/src/addons/calendar/services/calendar.ts +++ b/src/addons/calendar/services/calendar.ts @@ -1809,6 +1809,7 @@ export type AddonCalendarEventBase = { key: string; // Key. component: string; // Component. alttext: string; // Alttext. + iconurl?: string; // @since 4.2. Icon image url. }; category?: { id: number; // Id. diff --git a/src/addons/mod/label/services/handlers/module.ts b/src/addons/mod/label/services/handlers/module.ts index 23209a2d1..9aea9b5a4 100644 --- a/src/addons/mod/label/services/handlers/module.ts +++ b/src/addons/mod/label/services/handlers/module.ts @@ -52,7 +52,7 @@ export class AddonModLabelModuleHandlerService extends CoreModuleHandlerBase imp module.description = ''; return { - icon: '', + icon: this.getIconSrc(), title, a11yTitle: '', class: 'addon-mod-label-handler', @@ -74,5 +74,12 @@ export class AddonModLabelModuleHandlerService extends CoreModuleHandlerBase imp return true; } + /** + * @inheritdoc + */ + getIconSrc(): string { + return ''; + } + } export const AddonModLabelModuleHandler = makeSingleton(AddonModLabelModuleHandlerService); diff --git a/src/addons/mod/lti/services/handlers/module.ts b/src/addons/mod/lti/services/handlers/module.ts index 953e4264f..1494456b3 100644 --- a/src/addons/mod/lti/services/handlers/module.ts +++ b/src/addons/mod/lti/services/handlers/module.ts @@ -58,10 +58,6 @@ export class AddonModLtiModuleHandlerService extends CoreModuleHandlerBase imple ): Promise { const data = await super.getData(module, courseId, sectionId, forCoursePage); data.showDownloadButton = false; - - // Handle custom icons. - data.icon = module.modicon; - data.buttons = [{ icon: 'fas-external-link-alt', label: 'addon.mod_lti.launchactivity', @@ -83,6 +79,13 @@ export class AddonModLtiModuleHandlerService extends CoreModuleHandlerBase imple return AddonModLtiIndexComponent; } + /** + * @inheritdoc + */ + getIconSrc(module?: CoreCourseModuleData | undefined, modicon?: string | undefined): string | undefined { + return module?.modicon ?? modicon ?? CoreCourse.getModuleIconSrc(this.modName); + } + } export const AddonModLtiModuleHandler = makeSingleton(AddonModLtiModuleHandlerService); diff --git a/src/core/features/course/classes/module-base-handler.ts b/src/core/features/course/classes/module-base-handler.ts index 088def419..6c278c502 100644 --- a/src/core/features/course/classes/module-base-handler.ts +++ b/src/core/features/course/classes/module-base-handler.ts @@ -41,7 +41,7 @@ export class CoreModuleHandlerBase implements Partial { forCoursePage?: boolean, // eslint-disable-line @typescript-eslint/no-unused-vars ): Promise | CoreCourseModuleHandlerData { return { - icon: CoreCourse.getModuleIconSrc(module.modname, module.modicon), + icon: this.getIconSrc(module, module.modicon), title: module.name, class: 'addon-mod_' + module.modname + '-handler', showDownloadButton: true, @@ -78,4 +78,15 @@ export class CoreModuleHandlerBase implements Partial { await CoreNavigator.navigateToSitePath(this.pageName + routeParams, options); } + /** + * @inheritdoc + */ + getIconSrc(module?: CoreCourseModuleData, modicon?: string): Promise | string | undefined { + if (!module) { + return modicon; + } + + return CoreCourse.getModuleIconSrc(module.name, modicon); + } + } diff --git a/src/core/features/course/services/module-delegate.ts b/src/core/features/course/services/module-delegate.ts index bbd3e5568..f8dead8d3 100644 --- a/src/core/features/course/services/module-delegate.ts +++ b/src/core/features/course/services/module-delegate.ts @@ -82,9 +82,10 @@ export interface CoreCourseModuleHandler extends CoreDelegateHandler { * Get the icon src for the module. * * @param module Module to get the icon from. + * @param modicon The mod icon string. * @returns The icon src. */ - getIconSrc?(module?: CoreCourseModuleData): Promise | string | undefined; + getIconSrc?(module?: CoreCourseModuleData, modicon?: string): Promise | string | undefined; /** * Check if this type of module supports a certain feature. @@ -390,9 +391,9 @@ export class CoreCourseModuleDelegateService extends CoreDelegate { - const icon = await this.executeFunctionOnEnabled>(modname, 'getIconSrc', [module]); + const icon = await this.executeFunctionOnEnabled>(modname, 'getIconSrc', [module, modicon]); - return icon || CoreCourse.getModuleIconSrc(modname, modicon) || ''; + return icon ?? CoreCourse.getModuleIconSrc(modname, modicon) ?? ''; } /** diff --git a/src/core/features/grades/pages/course/course.page.ts b/src/core/features/grades/pages/course/course.page.ts index 9b1608047..c70adc1a8 100644 --- a/src/core/features/grades/pages/course/course.page.ts +++ b/src/core/features/grades/pages/course/course.page.ts @@ -206,7 +206,7 @@ export class CoreGradesCoursePage implements AfterViewInit, OnDestroy { */ private async fetchGrades(): Promise { const table = await CoreGrades.getCourseGradesTable(this.courseId, this.userId); - const formattedTable = CoreGradesHelper.formatGradesTable(table); + const formattedTable = await CoreGradesHelper.formatGradesTable(table); this.title = formattedTable.rows[0]?.gradeitem ?? Translate.instant('core.grades.grades'); this.columns = formattedTable.columns; diff --git a/src/core/features/grades/services/grades-helper.ts b/src/core/features/grades/services/grades-helper.ts index 50e9cef5a..fb5d4b3f3 100644 --- a/src/core/features/grades/services/grades-helper.ts +++ b/src/core/features/grades/services/grades-helper.ts @@ -37,6 +37,7 @@ import { makeSingleton, Translate } from '@singletons'; import { CoreError } from '@classes/errors/error'; import { CoreCourseHelper } from '@features/course/services/course-helper'; import { CoreAppProvider } from '@services/app'; +import { CoreCourseModuleDelegate } from '@features/course/services/module-delegate'; export const GRADES_PAGE_NAME = 'grades'; @@ -73,7 +74,7 @@ export class CoreGradesHelperProvider { let content = String(column.content); if (name == 'itemname') { - this.setRowIconAndType(row, content); + await this.setRowIconAndType(row, content); row.link = this.getModuleLink(content); row.rowclass += column.class.indexOf('hidden') >= 0 ? ' hidden' : ''; @@ -102,7 +103,10 @@ export class CoreGradesHelperProvider { * @param useLegacyLayout Whether to use the layout before 4.1. * @returns Formatted row object. */ - protected formatGradeRowForTable(tableRow: CoreGradesTableRow, useLegacyLayout: boolean): CoreGradesFormattedTableRow { + protected async formatGradeRowForTable( + tableRow: CoreGradesTableRow, + useLegacyLayout: boolean, + ): Promise { const row: CoreGradesFormattedTableRow = {}; if (!useLegacyLayout && 'leader' in tableRow) { @@ -132,7 +136,7 @@ export class CoreGradesHelperProvider { row.colspan = itemNameColumn.colspan; row.rowspan = tableRow.leader?.rowspan || 1; - this.setRowIconAndType(row, content); + await this.setRowIconAndType(row, content); this.setRowStyleClasses(row, itemNameColumn.class); row.rowclass += itemNameColumn.class.indexOf('hidden') >= 0 ? ' hidden' : ''; row.rowclass += itemNameColumn.class.indexOf('dimmed_text') >= 0 ? ' dimmed_text' : ''; @@ -203,7 +207,7 @@ export class CoreGradesHelperProvider { * @param table JSON object representing a table with data. * @returns Formatted HTML table. */ - formatGradesTable(table: CoreGradesTable): CoreGradesFormattedTable { + async formatGradesTable(table: CoreGradesTable): Promise { const maxDepth = table.maxdepth; const formatted: CoreGradesFormattedTable = { columns: [], @@ -223,7 +227,7 @@ export class CoreGradesHelperProvider { feedback: false, contributiontocoursetotal: false, }; - formatted.rows = this.formatGradesTableRows(table.tabledata); + formatted.rows = await this.formatGradesTableRows(table.tabledata); // Get a row with some info. let normalRow = formatted.rows.find( @@ -261,9 +265,9 @@ export class CoreGradesHelperProvider { * @param rows Unformatted rows. * @returns Formatted rows. */ - protected formatGradesTableRows(rows: CoreGradesTableRow[]): CoreGradesFormattedTableRow[] { + protected async formatGradesTableRows(rows: CoreGradesTableRow[]): Promise { const useLegacyLayout = !CoreSites.getRequiredCurrentSite().isVersionGreaterEqualThan('4.1'); - const formattedRows = rows.map(row => this.formatGradeRowForTable(row, useLegacyLayout)); + const formattedRows = await Promise.all(rows.map(row => this.formatGradeRowForTable(row, useLegacyLayout))); if (!useLegacyLayout) { for (let index = 0; index < formattedRows.length - 1; index++) { @@ -652,7 +656,7 @@ export class CoreGradesHelperProvider { * @param row Row. * @param text Row content. */ - protected setRowIconAndType(row: CoreGradesFormattedRowCommonData, text: string): void { + protected async setRowIconAndType(row: CoreGradesFormattedRowCommonData, text: string): Promise { text = text.replace('%2F', '/').replace('%2f', '/'); if (text.indexOf('/agg_mean') > -1) { row.itemtype = 'agg_mean'; @@ -684,7 +688,7 @@ export class CoreGradesHelperProvider { row.itemtype = 'mod'; row.itemmodule = module[1]; row.iconAlt = CoreCourse.translateModuleName(row.itemmodule) || ''; - row.image = CoreCourse.getModuleIconSrc( + row.image = await CoreCourseModuleDelegate.getModuleIconSrc( module[1], CoreDomUtils.convertToElement(text).querySelector('img')?.getAttribute('src') ?? undefined, ); From f82d3504f6f6ccfba015e4144be4611f1b06a0fa Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Tue, 14 Mar 2023 18:20:11 +0100 Subject: [PATCH 2/2] MOBILE-4193 grades: Avoid filtering rich images --- .../mod/lti/services/handlers/module.ts | 14 +++++++++++++ src/core/components/mod-icon/mod-icon.html | 4 ++-- src/core/components/mod-icon/mod-icon.scss | 5 +++++ src/core/components/mod-icon/mod-icon.ts | 7 ++++++- .../course/services/module-delegate.ts | 21 +++++++++++++++++++ .../features/grades/pages/course/course.html | 3 ++- .../grades/pages/course/course.page.ts | 9 ++++++++ .../features/grades/services/grades-helper.ts | 15 +++++++------ 8 files changed, 68 insertions(+), 10 deletions(-) diff --git a/src/addons/mod/lti/services/handlers/module.ts b/src/addons/mod/lti/services/handlers/module.ts index 1494456b3..545aa262f 100644 --- a/src/addons/mod/lti/services/handlers/module.ts +++ b/src/addons/mod/lti/services/handlers/module.ts @@ -22,6 +22,7 @@ import { AddonModLtiHelper } from '../lti-helper'; import { AddonModLtiIndexComponent } from '../../components/index'; import { CoreModuleHandlerBase } from '@features/course/classes/module-base-handler'; import { CoreCourse } from '@features/course/services/course'; +import { CoreSites } from '@services/sites'; /** * Handler to support LTI modules. @@ -86,6 +87,19 @@ export class AddonModLtiModuleHandlerService extends CoreModuleHandlerBase imple return module?.modicon ?? modicon ?? CoreCourse.getModuleIconSrc(this.modName); } + /** + * @inheritdoc + */ + iconIsShape(module?: CoreCourseModuleData | undefined, modicon?: string | undefined): boolean | undefined { + const iconUrl = module?.modicon ?? modicon; + + if (!iconUrl) { + return true; + } + + return iconUrl.startsWith(CoreSites.getRequiredCurrentSite().siteUrl); + } + } export const AddonModLtiModuleHandler = makeSingleton(AddonModLtiModuleHandlerService); diff --git a/src/core/components/mod-icon/mod-icon.html b/src/core/components/mod-icon/mod-icon.html index 58842cb13..46777347a 100644 --- a/src/core/components/mod-icon/mod-icon.html +++ b/src/core/components/mod-icon/mod-icon.html @@ -1,5 +1,5 @@ + class="core-module-icon" [class.no-filter]="noFilter" (error)="loadFallbackIcon()"> diff --git a/src/core/components/mod-icon/mod-icon.scss b/src/core/components/mod-icon/mod-icon.scss index f5f7dbdcb..5ab0db1f0 100644 --- a/src/core/components/mod-icon/mod-icon.scss +++ b/src/core/components/mod-icon/mod-icon.scss @@ -42,6 +42,11 @@ img { white-space: nowrap; overflow: hidden; } + + &.no-filter { + --filter: none; + } + } :host-context(ion-item) { diff --git a/src/core/components/mod-icon/mod-icon.ts b/src/core/components/mod-icon/mod-icon.ts index f2e59c7d7..1d2f018b2 100644 --- a/src/core/components/mod-icon/mod-icon.ts +++ b/src/core/components/mod-icon/mod-icon.ts @@ -13,7 +13,7 @@ // limitations under the License. import { CoreConstants, ModPurpose } from '@/core/constants'; -import { Component, ElementRef, Input, OnChanges, OnInit, SimpleChange } from '@angular/core'; +import { Component, ElementRef, EventEmitter, Input, OnChanges, OnInit, Output, SimpleChange } from '@angular/core'; import { CoreCourse } from '@features/course/services/course'; import { CoreCourseModuleDelegate } from '@features/course/services/module-delegate'; import { CoreSites } from '@services/sites'; @@ -34,9 +34,12 @@ export class CoreModIconComponent implements OnInit, OnChanges { @Input() modname?: string; // The module name. Used also as component if set. @Input() componentId?: number; // Component Id for external icons. @Input() modicon?: string; // Module icon url or local url. + @Input() noFilter?: boolean; // Whether to disable filters. @Input() showAlt = true; // Show alt otherwise it's only presentation icon. @Input() purpose: ModPurpose = ModPurpose.MOD_PURPOSE_OTHER; // Purpose of the module. + @Output() failedLoading = new EventEmitter(); + icon = ''; modNameTranslated = ''; isLocalUrl = true; @@ -122,6 +125,8 @@ export class CoreModIconComponent implements OnInit, OnChanges { } this.icon = path + moduleName + '.svg'; + + this.failedLoading.emit(); } } diff --git a/src/core/features/course/services/module-delegate.ts b/src/core/features/course/services/module-delegate.ts index f8dead8d3..0a6a2af3a 100644 --- a/src/core/features/course/services/module-delegate.ts +++ b/src/core/features/course/services/module-delegate.ts @@ -87,6 +87,15 @@ export interface CoreCourseModuleHandler extends CoreDelegateHandler { */ getIconSrc?(module?: CoreCourseModuleData, modicon?: string): Promise | string | undefined; + /** + * Check whether the icon should be treated as a shape or a rich image. + * + * @param module Module to get the icon from. + * @param modicon The mod icon string. + * @returns Whether the icon should be treated as a shape. + */ + iconIsShape?(module?: CoreCourseModuleData, modicon?: string): Promise | boolean | undefined; + /** * Check if this type of module supports a certain feature. * If this function is implemented, the supportedFeatures object will be ignored. @@ -396,6 +405,18 @@ export class CoreCourseModuleDelegateService extends CoreDelegate { + return await this.executeFunctionOnEnabled>(modname, 'iconIsShape', [module, modicon]); + } + /** * Check if a certain type of module supports a certain feature. * diff --git a/src/core/features/grades/pages/course/course.html b/src/core/features/grades/pages/course/course.html index 70cc02b61..44236689c 100644 --- a/src/core/features/grades/pages/course/course.html +++ b/src/core/features/grades/pages/course/course.html @@ -48,7 +48,8 @@ + [modname]="row.itemmodule" [noFilter]="row.imageIsShape === false" + (failedLoading)="failedLoadingRowImage(row)"> diff --git a/src/core/features/grades/pages/course/course.page.ts b/src/core/features/grades/pages/course/course.page.ts index c70adc1a8..508d99488 100644 --- a/src/core/features/grades/pages/course/course.page.ts +++ b/src/core/features/grades/pages/course/course.page.ts @@ -239,4 +239,13 @@ export class CoreGradesCoursePage implements AfterViewInit, OnDestroy { infiniteComplete && infiniteComplete(); } + /** + * Handle row image failed loading. + * + * @param row Row data. + */ + failedLoadingRowImage(row: CoreGradesFormattedTableRow): void { + delete row.imageIsShape; + } + } diff --git a/src/core/features/grades/services/grades-helper.ts b/src/core/features/grades/services/grades-helper.ts index fb5d4b3f3..96c4ea06e 100644 --- a/src/core/features/grades/services/grades-helper.ts +++ b/src/core/features/grades/services/grades-helper.ts @@ -684,14 +684,16 @@ export class CoreGradesHelperProvider { row.iconAlt = Translate.instant('core.grades.calculatedgrade'); } else if (text.indexOf('/mod/') > -1) { const module = text.match(/mod\/([^/]*)\//); - if (module?.[1] !== undefined) { + const modname = module?.[1]; + + if (modname !== undefined) { + const modicon = CoreDomUtils.convertToElement(text).querySelector('img')?.getAttribute('src') ?? undefined; + row.itemtype = 'mod'; - row.itemmodule = module[1]; + row.itemmodule = modname; row.iconAlt = CoreCourse.translateModuleName(row.itemmodule) || ''; - row.image = await CoreCourseModuleDelegate.getModuleIconSrc( - module[1], - CoreDomUtils.convertToElement(text).querySelector('img')?.getAttribute('src') ?? undefined, - ); + row.image = await CoreCourseModuleDelegate.getModuleIconSrc(modname, modicon); + row.imageIsShape = await CoreCourseModuleDelegate.moduleIconIsShape(modname, modicon); } } else { if (row.rowspan && row.rowspan > 1) { @@ -804,6 +806,7 @@ export type CoreGradesFormattedRowCommonData = { rowclass?: string; itemtype?: string; image?: string; + imageIsShape?: boolean; itemmodule?: string; iconAlt?: string; rowspan?: number;