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

implementation/59434 visualize reminders in notification center #17323

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions app/menus/notifications/menu.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def inbox_menu
end

def reason_filters
%w[mentioned assigned responsible watched dateAlert shared].map do |reason|
%w[mentioned assigned responsible watched dateAlert shared reminder].map do |reason|
count = unread_by_reason[reason]
menu_item(title: I18n.t("notifications.reasons.#{reason}"),
icon_key: reason,
Expand Down Expand Up @@ -128,7 +128,8 @@ def icon_map
"responsible" => :"op-person-accountable",
"watched" => :eye,
"shared" => :"share-android",
"dateAlert" => :"op-calendar-alert"
"dateAlert" => :"op-calendar-alert",
"reminder" => :clock
}
end

Expand Down
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,7 @@ en:
login: "Username"
mail: "Email"
name: "Name"
note: "Note"
password: "Password"
priority: "Priority"
project: "Project"
Expand Down Expand Up @@ -2203,6 +2204,7 @@ en:
responsible: "Accountable"
shared: "Shared"
watched: "Watcher"
reminder: "Reminder"
facets:
unread: "Unread"
unread_title: "Show unread"
Expand Down
2 changes: 2 additions & 0 deletions config/locales/js-en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,8 @@ en:
new_notifications:
message: "There are new notifications."
link_text: "Click here to load them."
reminders:
note: "Note: “%{note}”"
settings:
change_notification_settings: 'You can modify your <a target="_blank" href="%{url}">notification settings</a> to ensure you never miss an important update.'
title: "Notification settings"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface IInAppNotificationHalResourceLinks extends IHalResourceLinks {
activity:IHalResourceLink;
}

export type IInAppNotificationDetailsAttribute = 'startDate'|'dueDate'|'date';
export type IInAppNotificationDetailsAttribute = 'startDate'|'dueDate'|'date'|'note';

export interface IInAppNotificationDetailsResource {
property:IInAppNotificationDetailsAttribute;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,28 @@ export class IanCenterService extends UntilDestroyedMixin {
notifications$ = this
.aggregatedCenterNotifications$
.pipe(
map((items) => Object.values(items)),
map((items) => {
return Object.values(items).reduce((acc, workPackageNotificationGroup) => {
const { reminders, others } = workPackageNotificationGroup.reduce((result, notification) => {
if (notification.reason === 'reminder') {
result.reminders.push(notification);
} else {
result.others.push(notification);
}
return result;
}, { reminders: [] as INotification[], others: [] as INotification[] });

// Extract reminders into standalone groups so they can be displayed individually
if (reminders.length > 0) {
reminders.forEach((reminder) => acc.push([reminder]));
}
if (others.length > 0) {
acc.push(others);
}

return acc;
}, [] as INotification[][]);
}),
distinctUntilChanged(),
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<div
class="op-ian-actors--date"
[title]="fixedTime"
[textContent]="relativeTime$ | async"
></div>
<op-in-app-notification-relative-time
[notification]="notification"
/>
<div class="op-ian-actors--container">
<ng-container *ngFor="let actor of actors | slice:0:3; let idx = index; let last = last">
<span *ngIf="last && actors.length > 1 && actors.length < 4" textContent=" {{ text.and }} "></span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@
align-items: center
color: var(--fgColor-muted)

&--date
@include text-shortener
max-width: 100%
line-height: 1rem

&--container
@include text-shortener
display: flex
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import { ChangeDetectionStrategy, Component, HostBinding, Input, OnInit, ViewEncapsulation } from '@angular/core';
import { DeviceService } from 'core-app/core/browser/device.service';
import { I18nService } from 'core-app/core/i18n/i18n.service';
import { INotification } from 'core-app/core/state/in-app-notifications/in-app-notification.model';
import { PrincipalLike } from 'core-app/shared/components/principal/principal-types';
import { Observable, timer } from 'rxjs';
import { distinctUntilChanged, map } from 'rxjs/operators';
import { I18nService } from 'core-app/core/i18n/i18n.service';
import { TimezoneService } from 'core-app/core/datetime/timezone.service';
import { DeviceService } from 'core-app/core/browser/device.service';

@Component({
selector: 'op-in-app-notification-actors-line',
Expand All @@ -24,13 +21,6 @@ export class InAppNotificationActorsLineComponent implements OnInit {
// The actor, if any
actors:PrincipalLike[] = [];

// Fixed notification time
fixedTime:string;

// Format relative elapsed time (n seconds/minutes/hours ago)
// at an interval for auto updating
relativeTime$:Observable<string>;

text = {
and: this.I18n.t('js.notifications.center.label_actor_and'),
and_other_singular: this.I18n.t('js.notifications.center.and_more_users.one'),
Expand All @@ -41,21 +31,14 @@ export class InAppNotificationActorsLineComponent implements OnInit {
loading: this.I18n.t('js.ajax.loading'),
placeholder: this.I18n.t('js.placeholders.default'),
mark_as_read: this.I18n.t('js.notifications.center.mark_as_read'),
updated_by_at: (age:string):string => this.I18n.t(
'js.notifications.center.text_update_date_by',
{ date: age },
),
};

constructor(
readonly deviceService:DeviceService,
private I18n:I18nService,
private timezoneService:TimezoneService,
) { }

ngOnInit():void {
this.buildTime();

// Don't show the actor if the first item is actor-less (date alert)
if (this.notification._links.actor) {
this.buildActors();
Expand All @@ -70,22 +53,6 @@ export class InAppNotificationActorsLineComponent implements OnInit {
return this.text.and_other_plural(number);
}

private buildTime() {
this.fixedTime = this.timezoneService.formattedDatetime(this.notification.createdAt);
this.relativeTime$ = timer(0, 10000)
.pipe(
map(() => {
const time = this.timezoneService.formattedRelativeDateTime(this.notification.createdAt);
if (this.notification._links.actor) {
return this.text.updated_by_at(time);
}

return time;
}),
distinctUntilChanged(),
);
}

private buildActors() {
const actors = this
.aggregatedNotifications
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import {
OnInit,
ViewEncapsulation,
} from '@angular/core';
import { INotification } from 'core-app/core/state/in-app-notifications/in-app-notification.model';
import { I18nService } from 'core-app/core/i18n/i18n.service';
import { TimezoneService } from 'core-app/core/datetime/timezone.service';
import { I18nService } from 'core-app/core/i18n/i18n.service';
import { IInAppNotificationDetailsAttribute, INotification } from 'core-app/core/state/in-app-notifications/in-app-notification.model';
import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource';
import * as moment from 'moment';
import { Moment } from 'moment';
Expand Down Expand Up @@ -49,6 +49,7 @@ export class InAppNotificationDateAlertComponent implements OnInit {
dueDate: this.I18n.t('js.work_packages.properties.dueDate'),
date: this.I18n.t('js.notifications.date_alerts.milestone_date'),
due_today: this.I18n.t('js.notifications.date_alerts.property_today'),
note: '', // date alerts do not have notes
};

constructor(
Expand All @@ -71,7 +72,7 @@ export class InAppNotificationDateAlertComponent implements OnInit {
}
}

private deriveDueDate(value:string, property:'startDate'|'dueDate'|'date') {
private deriveDueDate(value:string, property:IInAppNotificationDetailsAttribute) {
const dateValue = this.timezoneService.parseISODate(value).startOf('day');
const today = moment();
this.dateIsPast = dateValue.isBefore(today, 'day');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,22 @@
<div
class="op-ian-item--bottom-line"
>
<op-in-app-notification-date-alert
*ngIf="showDateAlert"
[workPackage]="workPackage"
<op-in-app-notification-reminder-alert
*ngIf="hasReminderAlert"
[aggregatedNotifications]="aggregatedNotifications"
></op-in-app-notification-date-alert>
<op-in-app-notification-actors-line
*ngIf="!showDateAlert"
[notification]="notification"
[aggregatedNotifications]="aggregatedNotifications"
></op-in-app-notification-actors-line>
/>
<ng-container *ngIf="!hasReminderAlert">
<op-in-app-notification-date-alert
*ngIf="showDateAlert"
[workPackage]="workPackage"
[aggregatedNotifications]="aggregatedNotifications"
></op-in-app-notification-date-alert>
<op-in-app-notification-actors-line
*ngIf="!showDateAlert"
[notification]="notification"
[aggregatedNotifications]="aggregatedNotifications"
></op-in-app-notification-actors-line>
</ng-container>
</div>
</ng-container>
<ng-template #workPackageLoading>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export class InAppNotificationEntryComponent implements OnInit {
workPackage$:Observable<WorkPackageResource>|null = null;

showDateAlert = false;
hasReminderAlert = false;

loading$ = this.storeService.query.selectLoading();

Expand Down Expand Up @@ -62,6 +63,7 @@ export class InAppNotificationEntryComponent implements OnInit {
const href = this.notification._links.resource?.href;
this.workPackageId = href && HalResource.matchFromLink(href, 'work_packages');

this.hasReminderAlert = this.aggregatedNotifications.some((notification) => notification.reason === 'reminder');
this.showDateAlert = this.hasActiveDateAlert();
this.buildTranslatedReason();
this.buildProject();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<div
class="op-ian-relative-time"
[title]="fixedTime"
[textContent]="relativeTime$ | async">
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@import "helpers"

.op-ian-relative-time
@include text-shortener
max-width: 100%
line-height: 1rem
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import {
ChangeDetectionStrategy,
Component,
Input,
OnInit,
ViewEncapsulation,
} from '@angular/core';
import { TimezoneService } from 'core-app/core/datetime/timezone.service';
import { I18nService } from 'core-app/core/i18n/i18n.service';
import { INotification } from 'core-app/core/state/in-app-notifications/in-app-notification.model';
import { Observable, timer } from 'rxjs';
import { distinctUntilChanged, map } from 'rxjs/operators';

@Component({
selector: 'op-in-app-notification-relative-time',
templateUrl: './in-app-notification-relative-time.component.html',
styleUrls: ['./in-app-notification-relative-time.component.sass'],
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None,
})
export class InAppNotificationRelativeTimeComponent implements OnInit {
@Input() notification:INotification;
@Input() hasActorByLine:boolean = true;

// Fixed notification time
fixedTime:string;

// Format relative elapsed time (n seconds/minutes/hours ago)
// at an interval for auto updating
relativeTime$:Observable<string>;

text = {
updated_by_at: (age:string):string => this.I18n.t(
'js.notifications.center.text_update_date_by',
{ date: age },
),
};

constructor(
private I18n:I18nService,
private timezoneService:TimezoneService,
) { }

ngOnInit():void {
this.buildTime();
}

private buildTime() {
this.fixedTime = this.timezoneService.formattedDatetime(this.notification.createdAt);
this.relativeTime$ = timer(0, 10000)
.pipe(
map(() => {
const time = this.timezoneService.formattedRelativeDateTime(this.notification.createdAt);
if (this.hasActorByLine && this.notification._links.actor) {
return this.text.updated_by_at(time);
}

return `${time}.`;
}),
distinctUntilChanged(),
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<op-in-app-notification-relative-time
[notification]="reminderAlert"
[hasActorByLine]="false"
/>
<span
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bildschirmfoto 2024-12-11 um 13 25 51

I still see the issue raised by wieland in our last meeting as well - a longer note text gets hidden on mobile

Copy link
Member Author

@akabiru akabiru Dec 11, 2024

Choose a reason for hiding this comment

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

Thanks! I had planned on resolving separately in https://community.openproject.org/wp/59989 but it should also be small enough to fit here 👍🏾

Copy link
Member Author

Choose a reason for hiding this comment

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

class="op-ian-reminder-alert--note"
[textContent]="reminderNote"
></span>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@import "helpers"

.op-ian-reminder-alert
display: grid
grid-template-columns: auto 1fr
grid-column-gap: $spot-spacing-0_25
align-items: center
color: var(--fgColor-muted)

&--note
@include text-shortener
line-height: 1rem
color: var(--fgColor-default)

> *
flex-shrink: 0
Loading
Loading