From 200d1c712bba604fadd1e2ccd57c09e01c857920 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Mon, 13 Jun 2022 09:37:20 +0200 Subject: [PATCH 1/4] [#41849] Adapt the style of the attachments section to the new designs - https://community.openproject.org/work_packages/41849 --- .../state/attachments/attachment.model.ts | 1 + .../core/state/resource-collection.service.ts | 11 +-- .../attachment-list-item.component.ts | 99 +++++++++++++------ .../attachment-list/attachment-list-item.html | 78 ++++++++------- .../attachment-list/attachment-list.html | 22 +++-- .../file-link-list-item.component.ts | 2 +- .../file-link-list/file-link-list-item.html | 1 + .../components/principal/principal-helper.ts | 5 +- .../content/work_packages/tabs/_files.sass | 6 +- 9 files changed, 137 insertions(+), 88 deletions(-) diff --git a/frontend/src/app/core/state/attachments/attachment.model.ts b/frontend/src/app/core/state/attachments/attachment.model.ts index bc04e5bffe..ea03f4b5a5 100644 --- a/frontend/src/app/core/state/attachments/attachment.model.ts +++ b/frontend/src/app/core/state/attachments/attachment.model.ts @@ -39,6 +39,7 @@ export interface IAttachmentHalResourceLinks extends IHalResourceLinks { container:IHalResourceLink; author:IHalResourceLink; downloadLocation:IHalResourceLink; + staticDownloadLocation:IHalResourceLink; } export interface IAttachment { diff --git a/frontend/src/app/core/state/resource-collection.service.ts b/frontend/src/app/core/state/resource-collection.service.ts index 4f56cca6c4..da377a0f54 100644 --- a/frontend/src/app/core/state/resource-collection.service.ts +++ b/frontend/src/app/core/state/resource-collection.service.ts @@ -84,16 +84,11 @@ export abstract class ResourceCollectionService { } /** - * Lookup a single entity from the store + * Checks, if the store already has a resource loaded by id. * @param id */ - lookupMultiple(id:ID):Observable { - return this - .query - .selectEntity(id) - .pipe( - filter((entity) => entity !== undefined), - ) as Observable; + exists(id:ID):boolean { + return this.query.hasEntity(id); } /** diff --git a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.component.ts b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.component.ts index 07b6a04141..f09dbc8e4f 100644 --- a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.component.ts +++ b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.component.ts @@ -27,34 +27,43 @@ //++ import { - ChangeDetectionStrategy, Component, EventEmitter, Input, OnInit, Output, + AfterViewInit, + ChangeDetectionStrategy, + Component, + ElementRef, + EventEmitter, + Input, + OnInit, + Output, + ViewChild, } from '@angular/core'; -import { Observable, of } from 'rxjs'; -import { map, switchMap } from 'rxjs/operators'; +import { BehaviorSubject, combineLatest, Observable } from 'rxjs'; +import { distinctUntilChanged, first } from 'rxjs/operators'; import { I18nService } from 'core-app/core/i18n/i18n.service'; -import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; -import { HalResource } from 'core-app/features/hal/resources/hal-resource'; +import { IPrincipal } from 'core-app/core/state/principals/principal.model'; import { IAttachment } from 'core-app/core/state/attachments/attachment.model'; +import { TimezoneService } from 'core-app/core/datetime/timezone.service'; +import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; +import { UntilDestroyedMixin } from 'core-app/shared/helpers/angular/until-destroyed.mixin'; import { PrincipalsResourceService } from 'core-app/core/state/principals/principals.service'; -import { IUser } from 'core-app/core/state/principals/user.model'; +import { PrincipalRendererService } from 'core-app/shared/components/principal/principal-renderer.service'; import idFromLink from 'core-app/features/hal/helpers/id-from-link'; @Component({ - selector: 'op-attachment-list-item', + // eslint-disable-next-line @angular-eslint/component-selector + selector: '[op-attachment-list-item]', templateUrl: './attachment-list-item.html', changeDetection: ChangeDetectionStrategy.OnPush, }) -export class AttachmentListItemComponent implements OnInit { - @Input() public resource:HalResource; - +export class AttachmentListItemComponent extends UntilDestroyedMixin implements OnInit, AfterViewInit { @Input() public attachment:IAttachment; @Input() public index:number; - @Input() destroyImmediately = true; - @Output() public removeAttachment = new EventEmitter(); + @ViewChild('avatar') avatar:ElementRef; + static imageFileExtensions:string[] = ['jpeg', 'jpg', 'gif', 'bmp', 'png']; public text = { @@ -67,23 +76,53 @@ export class AttachmentListItemComponent implements OnInit { return this.text.removeFile({ fileName: this.attachment.fileName }); } - public author$:Observable; + public author$:Observable; + + public timestampText:string; + + private viewInitialized$ = new BehaviorSubject(false); - constructor(private readonly principalsResourceService:PrincipalsResourceService, + constructor( private readonly I18n:I18nService, - private readonly pathHelper:PathHelperService) { + private readonly pathHelper:PathHelperService, + private readonly timezoneService:TimezoneService, + private readonly principalsResourceService:PrincipalsResourceService, + private readonly principalRendererService:PrincipalRendererService, + ) { + super(); } ngOnInit():void { const authorId = idFromLink(this.attachment._links.author.href); - this.author$ = this - .principalsResourceService - .lookup(authorId) - .pipe( - switchMap((user) => (user ? of(user) : this.principalsResourceService.fetchUser(authorId))), - map((user) => user as IUser), - ); + if (!this.principalsResourceService.exists(authorId)) { + this.principalsResourceService.fetchUser(authorId).subscribe(); + } + + this.timestampText = this.timezoneService.parseDatetime(this.attachment.createdAt).fromNow(); + + this.author$ = this.principalsResourceService.lookup(authorId).pipe(first()); + + combineLatest([ + this.author$, + this.viewInitialized$.pipe(distinctUntilChanged()), + ]).pipe(this.untilDestroyed()) + .subscribe(([user, initialized]) => { + if (!initialized) { + return; + } + + this.principalRendererService.render( + this.avatar.nativeElement, + user, + { hide: true, link: false }, + { hide: false, size: 'mini' }, + ); + }); + } + + ngAfterViewInit():void { + this.viewInitialized$.next(true); } /** @@ -108,29 +147,25 @@ export class AttachmentListItemComponent implements OnInit { if (this.isImage) { el = document.createElement('img'); el.src = url; - el.textContent = this.fileName; + el.textContent = this.attachment.fileName; } else { el = document.createElement('a'); el.href = url; - el.textContent = this.fileName; + el.textContent = this.attachment.fileName; } return el; } - public get downloadPath():string { - return this.pathHelper.attachmentDownloadPath(String(this.attachment.id), this.fileName); + private get downloadPath():string { + return this.pathHelper.attachmentDownloadPath(String(this.attachment.id), this.attachment.fileName); } - public get isImage():boolean { - const ext = this.fileName.split('.').pop() || ''; + private get isImage():boolean { + const ext = this.attachment.fileName.split('.').pop() || ''; return AttachmentListItemComponent.imageFileExtensions.indexOf(ext.toLowerCase()) > -1; } - public get fileName():string { - return this.attachment.fileName; - } - public confirmRemoveAttachment($event:JQuery.TriggeredEvent):boolean { if (!window.confirm(this.text.destroyConfirmation)) { $event.stopImmediatePropagation(); diff --git a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html index 5532244719..a271bcfe5a 100644 --- a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html +++ b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html @@ -1,39 +1,47 @@ -
  • - - - +
    + + - {{ fileName }} + - - - - - + + +
    +
    + + + + +
    +
    - -
    • + + diff --git a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list.html b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list.html index da5752594d..e3d1e8eb98 100644 --- a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list.html +++ b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list.html @@ -1,10 +1,16 @@ -
    -
      - - +
      +
        +
      diff --git a/frontend/src/app/shared/components/file-links/file-link-list/file-link-list-item.component.ts b/frontend/src/app/shared/components/file-links/file-link-list/file-link-list-item.component.ts index d57c4aaad5..cd30a87e32 100644 --- a/frontend/src/app/shared/components/file-links/file-link-list/file-link-list-item.component.ts +++ b/frontend/src/app/shared/components/file-links/file-link-list/file-link-list-item.component.ts @@ -100,7 +100,7 @@ export class FileLinkListItemComponent implements OnInit, AfterViewInit { if (this.originData.lastModifiedByName) { this.principalRendererService.render( this.avatar.nativeElement, - { name: this.originData.lastModifiedByName, href: '/users/1' }, + { name: this.originData.lastModifiedByName, href: '/external_users/1' }, { hide: true, link: false }, { hide: false, size: 'mini' }, ); diff --git a/frontend/src/app/shared/components/file-links/file-link-list/file-link-list-item.html b/frontend/src/app/shared/components/file-links/file-link-list/file-link-list-item.html index 16ded077fe..2c76141a7b 100644 --- a/frontend/src/app/shared/components/file-links/file-link-list/file-link-list-item.html +++ b/frontend/src/app/shared/components/file-links/file-link-list/file-link-list-item.html @@ -11,6 +11,7 @@ diff --git a/frontend/src/app/shared/components/principal/principal-helper.ts b/frontend/src/app/shared/components/principal/principal-helper.ts index 4c70b1c991..4dac9c4867 100644 --- a/frontend/src/app/shared/components/principal/principal-helper.ts +++ b/frontend/src/app/shared/components/principal/principal-helper.ts @@ -30,8 +30,7 @@ import { PrincipalLike } from 'core-app/shared/components/principal/principal-ty import { IPrincipal } from 'core-app/core/state/principals/principal.model'; import { HalSourceLink } from 'core-app/features/hal/resources/hal-resource'; -export type PrincipalType = 'user'|'placeholder_user'|'group'; -export type PrincipalPluralType = 'users'|'placeholder_users'|'groups'; +export type PrincipalType = 'user'|'placeholder_user'|'group'|'external_user'; /* * This function is a helper that wraps around the old HalResource based principal type and the new interface based one. @@ -51,7 +50,7 @@ export function hrefFromPrincipal(p:IPrincipal|PrincipalLike):string { return ''; } export function typeFromHref(href:string):PrincipalType|null { - const match = /\/(user|group|placeholder_user)s\/\d+$/.exec(href); + const match = /\/(user|group|placeholder_user|external_user)s\/\d+$/.exec(href); if (!match) { return null; diff --git a/frontend/src/global_styles/content/work_packages/tabs/_files.sass b/frontend/src/global_styles/content/work_packages/tabs/_files.sass index 72a8171d1d..2c01b947ad 100644 --- a/frontend/src/global_styles/content/work_packages/tabs/_files.sass +++ b/frontend/src/global_styles/content/work_packages/tabs/_files.sass @@ -26,7 +26,8 @@ // See COPYRIGHT and LICENSE files for more details. //++ -.op-files-tab +.op-files-tab, +.op-attachments-list .op-files-tab--file-list .op-files-tab--file-list-item .op-files-tab--file-list-item-action @@ -128,3 +129,6 @@ &_default color: #9A9A9A + + &_clip + color: #333333 From 53e2cecaf7f3c331c8b136463df6b7e5887dd72b Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Thu, 16 Jun 2022 15:11:20 +0200 Subject: [PATCH 2/4] [#41849] add data-qa-selector to new attachment html --- .../attachment-list/attachment-list-item.html | 1 + .../spec/features/meetings_attachments_spec.rb | 11 ++++++----- spec/features/admin/attribute_help_texts_spec.rb | 2 +- .../attachments/attachment_upload_spec.rb | 9 +++++---- spec/features/work_packages/navigation_spec.rb | 3 ++- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html index a271bcfe5a..6d41e40009 100644 --- a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html +++ b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html @@ -13,6 +13,7 @@ [textContent]="attachment.fileName" [title]="attachment.fileName" class="spot-list--item-title op-files-tab--file-list-item-title" + data-qa-selector="op-files-tab--file-list-item-title" > Date: Mon, 20 Jun 2022 10:54:46 +0200 Subject: [PATCH 3/4] [#41849] added some spacings after QA of UIX --- .../global_styles/content/work_packages/tabs/_files.sass | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frontend/src/global_styles/content/work_packages/tabs/_files.sass b/frontend/src/global_styles/content/work_packages/tabs/_files.sass index 2c01b947ad..5620f4a7f1 100644 --- a/frontend/src/global_styles/content/work_packages/tabs/_files.sass +++ b/frontend/src/global_styles/content/work_packages/tabs/_files.sass @@ -44,6 +44,7 @@ word-break: normal overflow: hidden text-overflow: ellipsis + padding-right: $spot_spacing-0-5 .op-files-tab--file-list-item-text @include spot-caption() @@ -73,6 +74,7 @@ &--storage-info-box margin-top: 0.875rem display: grid + align-items: center grid-template: "icon text" "button button" / auto 1fr .spot-icon.big @@ -81,6 +83,7 @@ width: 3.375rem height: 3.375rem font-size: 1.25rem + margin-right: $spot_spacing-0-5 .text-box grid-area: text @@ -99,6 +102,9 @@ display: flex justify-content: end + > .button_no-margin + margin-top: $spot_spacing-0-75 + &--icon &_pdf color: #B93A33 From bd3d6273897a819282e6189c1b4e2ee07d2fc242 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Mon, 20 Jun 2022 15:21:20 +0200 Subject: [PATCH 4/4] [#41849] added sorting to attachments list - replaced angular trackBy with own sorting inside observable - added file icons to attachment list items - fixed clip icon usage --- .../files-tab/op-files-tab.html | 3 ++- .../attachment-list-item.component.ts | 8 ++++++++ .../attachment-list/attachment-list-item.html | 3 ++- .../attachment-list.component.ts | 20 +++++++++++++------ .../attachment-list/attachment-list.html | 2 +- .../file-link-list-item-icon.factory.ts | 4 ++-- .../file-link-icons/icon-mappings.ts | 4 ++-- .../file-link-list-item.component.ts | 4 ++-- .../content/work_packages/tabs/_files.sass | 3 +++ .../work_packages/tabs/_tab_content.sass | 1 - 10 files changed, 36 insertions(+), 16 deletions(-) diff --git a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/files-tab/op-files-tab.html b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/files-tab/op-files-tab.html index 0b47d44507..ecf5b10d36 100644 --- a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/files-tab/op-files-tab.html +++ b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/files-tab/op-files-tab.html @@ -9,6 +9,7 @@ *ngIf="canViewFileLinks$ | async" class="op-tab-content--header" > +
    @@ -23,7 +24,7 @@ class="op-tab-content--tab-section" >
    - +
    diff --git a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.component.ts b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.component.ts index f09dbc8e4f..b48676a7ca 100644 --- a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.component.ts +++ b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.component.ts @@ -48,6 +48,10 @@ import { UntilDestroyedMixin } from 'core-app/shared/helpers/angular/until-destr import { PrincipalsResourceService } from 'core-app/core/state/principals/principals.service'; import { PrincipalRendererService } from 'core-app/shared/components/principal/principal-renderer.service'; import idFromLink from 'core-app/features/hal/helpers/id-from-link'; +import { IFileIcon } from 'core-app/shared/components/file-links/file-link-icons/icon-mappings'; +import { + getIconForMimeType, +} from 'core-app/shared/components/file-links/file-link-icons/file-link-list-item-icon.factory'; @Component({ // eslint-disable-next-line @angular-eslint/component-selector @@ -80,6 +84,8 @@ export class AttachmentListItemComponent extends UntilDestroyedMixin implements public timestampText:string; + public fileIcon:IFileIcon; + private viewInitialized$ = new BehaviorSubject(false); constructor( @@ -93,6 +99,8 @@ export class AttachmentListItemComponent extends UntilDestroyedMixin implements } ngOnInit():void { + this.fileIcon = getIconForMimeType(this.attachment.contentType); + const authorId = idFromLink(this.attachment._links.author.href); if (!this.principalsResourceService.exists(authorId)) { diff --git a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html index 6d41e40009..c6bc6a6503 100644 --- a/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html +++ b/frontend/src/app/shared/components/attachments/attachment-list/attachment-list-item.html @@ -6,7 +6,7 @@ (dragstart)="setDragData($event)" >