From 3f3e83057db3d9f36d1eec578bc3d99284c9f7f6 Mon Sep 17 00:00:00 2001 From: Denys Vuika Date: Mon, 31 Jul 2023 19:23:46 +0100 Subject: [PATCH] [ACS-5703] Comment List code and styles cleanup (#8787) * remove useless locale * remove useless id values, update tests * code cleanup * fix formatting * css cleanup * code cleanup * style cleanup * fix css scope * cleanup styles * remove sanitise and don't bind to innerHTML * reduce ng-container * move model specific logic to Comment Model * update tests, remove sanitise tests * drop carma coverage to 72 as code removed * drop selection animation as selection operations are not supported by the component itself * cleanup css * fix tests and lint * update stories and tests * fix line breaks * move e2e to unit test * disable search tests * disable search tests * disable search tests --- .../components/comment-component.e2e.ts | 33 ++------- e2e/protractor.excludes.json | 10 ++- .../node-comments/mocks/node-comments.mock.ts | 32 ++++----- lib/core/karma.conf.js | 2 +- .../comment-list/comment-list.component.html | 31 +++----- .../comment-list/comment-list.component.scss | 41 ++--------- .../comment-list.component.spec.ts | 44 +++--------- .../comment-list/comment-list.component.ts | 59 ++-------------- .../mocks/comment-list.service.mock.ts | 3 - .../src/lib/comments/comments.component.html | 16 ++--- .../src/lib/comments/comments.component.scss | 62 ++++++++-------- .../lib/comments/comments.component.spec.ts | 43 ++++++------ .../src/lib/comments/comments.component.ts | 42 ++--------- lib/core/src/lib/comments/interfaces/index.ts | 18 ----- .../src/lib/comments/interfaces/public-api.ts | 19 ----- .../comments/mocks/comments.service.mock.ts | 70 ++++++++++--------- .../mocks/comments.service.stories.mock.ts | 30 ++++---- .../comments/mocks/comments.stories.mock.ts | 48 ++++++------- lib/core/src/lib/comments/public-api.ts | 3 +- lib/core/src/lib/i18n/en.json | 1 + lib/core/src/lib/models/comment.model.ts | 27 +++++++ 21 files changed, 235 insertions(+), 399 deletions(-) delete mode 100644 lib/core/src/lib/comments/interfaces/index.ts delete mode 100644 lib/core/src/lib/comments/interfaces/public-api.ts diff --git a/e2e/content-services/components/comment-component.e2e.ts b/e2e/content-services/components/comment-component.e2e.ts index bea4c42ff42..f6542aa69e4 100644 --- a/e2e/content-services/components/comment-component.e2e.ts +++ b/e2e/content-services/components/comment-component.e2e.ts @@ -32,17 +32,17 @@ import CONSTANTS = require('../../util/constants'); import { SitesApi, SiteEntry, CommentsApi } from '@alfresco/js-api'; describe('Comment', () => { - - const loginPage: LoginPage = new LoginPage(); - const contentServicesPage: ContentServicesPage = new ContentServicesPage(); - const viewerPage: ViewerPage = new ViewerPage(); - const commentsPage: CommentsPage = new CommentsPage(); + const loginPage = new LoginPage(); + const contentServicesPage = new ContentServicesPage(); + const viewerPage = new ViewerPage(); + const commentsPage = new CommentsPage(); const navigationBarPage = new NavigationBarPage(); const apiService = createApiService(); const commentsApi = new CommentsApi(apiService.getInstance()); - let userFullName; let nodeId; + let userFullName: string; + let nodeId: string; let acsUser: UserModel; const pngFileModel = new FileModel({ @@ -57,11 +57,6 @@ describe('Comment', () => { first: 'This is a comment', multiline: 'This is a comment\n' + 'with a new line', second: 'This is another comment', - codeType: `
- First name:
- Last name:
- -
`, test: 'Test' }; @@ -149,22 +144,6 @@ describe('Comment', () => { await expect(await commentsPage.getUserName(0)).toEqual(userFullName); await expect(await commentsPage.getTime(0)).toMatch(/(ago|few)/); }); - - it('[C280022] Should treat HTML code as a regular string', async () => { - const resultStr = comments.codeType.replace(/\s\s+/g, ' '); - await viewerPage.viewFile(pngFileModel.name); - await viewerPage.clickInfoButton(); - await viewerPage.checkInfoSideBarIsDisplayed(); - await viewerPage.clickOnCommentsTab(); - - await commentsPage.addComment(comments.codeType); - await commentsPage.checkUserIconIsDisplayed(); - - await commentsPage.getTotalNumberOfComments('Comments (1)'); - await expect(await commentsPage.getMessage(0)).toEqual(resultStr); - await expect(await commentsPage.getUserName(0)).toEqual(userFullName); - await expect(await commentsPage.getTime(0)).toMatch(/(ago|few)/); - }); }); describe('Consumer Permissions', () => { diff --git a/e2e/protractor.excludes.json b/e2e/protractor.excludes.json index 3a2383ff6c8..20308007c74 100644 --- a/e2e/protractor.excludes.json +++ b/e2e/protractor.excludes.json @@ -13,5 +13,13 @@ "C280063": "https://alfresco.atlassian.net/browse/ACS-4595", "C280064": "https://alfresco.atlassian.net/browse/ACS-4595", "C280407": "https://alfresco.atlassian.net/browse/ACS-4595", - "C277288": "https://alfresco.atlassian.net/browse/AAE-15475" + "C277288": "https://alfresco.atlassian.net/browse/AAE-15475", + "C280054": "https://alfresco.atlassian.net/browse/ACS-5742", + "C280058": "https://alfresco.atlassian.net/browse/ACS-5742", + "C286298": "https://alfresco.atlassian.net/browse/ACS-5742", + "C277146": "https://alfresco.atlassian.net/browse/ACS-5742", + "C286556": "https://alfresco.atlassian.net/browse/ACS-5742", + "C291802": "https://alfresco.atlassian.net/browse/ACS-5742", + "C277280": "https://alfresco.atlassian.net/browse/ACS-5742", + "C277281": "https://alfresco.atlassian.net/browse/ACS-5742" } diff --git a/lib/content-services/src/lib/node-comments/mocks/node-comments.mock.ts b/lib/content-services/src/lib/node-comments/mocks/node-comments.mock.ts index 96978db8816..c608dee260e 100644 --- a/lib/content-services/src/lib/node-comments/mocks/node-comments.mock.ts +++ b/lib/content-services/src/lib/node-comments/mocks/node-comments.mock.ts @@ -149,63 +149,63 @@ export const testUser: EcmUserModel = { export const getDateXMinutesAgo = (minutes: number) => new Date(new Date().getTime() - minutes * 60000); export const commentsNodeData: CommentModel[] = [ - { + new CommentModel({ id: 1, message: `I've done this component, is it cool?`, created: getDateXMinutesAgo(30), createdBy: johnDoe, isSelected: false - }, - { + }), + new CommentModel({ id: 2, message: 'Yeah', created: getDateXMinutesAgo(15), createdBy: janeEod, isSelected: false - }, - { + }), + new CommentModel({ id: 3, message: '+1', created: getDateXMinutesAgo(12), createdBy: robertSmith, isSelected: false - }, - { + }), + new CommentModel({ id: 4, message: 'ty', created: new Date(), createdBy: johnDoe, isSelected: false - } + }) ]; export const commentsTaskData: CommentModel[] = [ - { + new CommentModel({ id: 1, message: `I've done this task, what's next?`, created: getDateXMinutesAgo(30), createdBy: johnDoe, isSelected: false - }, - { + }), + new CommentModel({ id: 2, message: `I've assigned you another one 🤠`, created: getDateXMinutesAgo(15), createdBy: janeEod, isSelected: false - }, - { + }), + new CommentModel({ id: 3, message: '+1', created: getDateXMinutesAgo(12), createdBy: robertSmith, isSelected: false - }, - { + }), + new CommentModel({ id: 4, message: 'Cheers', created: new Date(), createdBy: johnDoe, isSelected: false - } + }) ]; diff --git a/lib/core/karma.conf.js b/lib/core/karma.conf.js index 4c1aab8ba43..14258dd8af2 100644 --- a/lib/core/karma.conf.js +++ b/lib/core/karma.conf.js @@ -89,7 +89,7 @@ module.exports = function (config) { global: { statements: 75, branches: 67, - functions: 73, + functions: 72, lines: 75 } } diff --git a/lib/core/src/lib/comments/comment-list/comment-list.component.html b/lib/core/src/lib/comments/comment-list/comment-list.component.html index 89b2f566e07..0577eb0d04e 100644 --- a/lib/core/src/lib/comments/comment-list/comment-list.component.html +++ b/lib/core/src/lib/comments/comment-list/comment-list.component.html @@ -1,30 +1,17 @@ -
-
- {{getUserShortName(comment.createdBy)}} -
-
- -
+ class="adf-comment-list-item"> +
+
{{ comment.userInitials }}
+
-
- {{comment.createdBy?.firstName}} {{comment.createdBy?.lastName}} -
-
-
-
- {{ comment.created | adfTimeAgo: currentLocale }} -
+
{{ comment.userDisplayName }}
+
{{ comment.message }}
+
{{ comment.created | adfTimeAgo }}
diff --git a/lib/core/src/lib/comments/comment-list/comment-list.component.scss b/lib/core/src/lib/comments/comment-list/comment-list.component.scss index 905fbb297ff..37f7f144668 100644 --- a/lib/core/src/lib/comments/comment-list/comment-list.component.scss +++ b/lib/core/src/lib/comments/comment-list/comment-list.component.scss @@ -1,10 +1,5 @@ -.adf-is-selected { - background: var(--adf-theme-primary-100); -} - .adf { &-comment-img-container { - float: left; width: 40px; height: 100%; display: flex; @@ -13,28 +8,12 @@ } &-comment-list-item { - /* stylelint-disable */ white-space: initial; - /* stylelint-enable */ display: table-row-group; padding-top: 12px; overflow: hidden; height: 100% !important; - transition: background 0.8s; background-position: center; - - &:hover { - background: - var(--adf-theme-primary-100) - radial-gradient(circle, transparent 1%, var(--adf-theme-primary-100) 1%) - center/15000%; - } - - &:active { - background-color: var(--adf-theme-primary-300); - background-size: 100%; - transition: background 0s; - } } &-comment-user-icon { @@ -50,21 +29,16 @@ } &-comment-user-name { - float: left; - width: calc(100% - 10%); + width: 100%; padding: 2px 10px; font-weight: 600; font-size: var(--theme-body-1-font-size); } &-comment-message { - float: left; - width: calc(100% - 10px); + width: 100%; padding: 2px 10px; - font-style: italic; - /* stylelint-disable */ - white-space: initial !important; - /* stylelint-enable */ + white-space: pre-line !important; font-size: var(--theme-body-1-font-size); letter-spacing: -0.2px; line-height: 1.43; @@ -72,17 +46,16 @@ } &-comment-message-time { - float: left; - width: calc(100% - 10%); + margin-top: 5px; + width: 100%; padding: 2px 10px; - font-size: var(--theme-caption-font-size) !important; + font-size: var(--theme-caption-font-size); color: var(--adf-theme-foreground-text-color); } &-comment-contents { - width: calc(100% - 10px); + width: 100%; padding-top: 12px; - padding-bottom: 12px; padding-left: 5px; } diff --git a/lib/core/src/lib/comments/comment-list/comment-list.component.spec.ts b/lib/core/src/lib/comments/comment-list/comment-list.component.spec.ts index 64410963692..18a945ee92a 100644 --- a/lib/core/src/lib/comments/comment-list/comment-list.component.spec.ts +++ b/lib/core/src/lib/comments/comment-list/comment-list.component.spec.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; import { ComponentFixture, fakeAsync, TestBed } from '@angular/core/testing'; import { CommentModel } from '../../models/comment.model'; import { CommentListComponent } from './comment-list.component'; @@ -26,7 +25,6 @@ import { commentUserNoPictureDefined, commentUserPictureDefined, mockCommentOne, - mockCommentTwo, testUser } from './mocks/comment-list.mock'; import { CommentListServiceMock } from './mocks/comment-list.service.mock'; @@ -44,7 +42,6 @@ describe('CommentListComponent', () => { TranslateModule.forRoot(), CoreTestingModule ], - schemas: [CUSTOM_ELEMENTS_SCHEMA], providers: [ { provide: ADF_COMMENTS_SERVICE, @@ -64,7 +61,7 @@ describe('CommentListComponent', () => { }); it('should emit row click event', fakeAsync(() => { - commentList.comments = [Object.assign({}, mockCommentOne)]; + commentList.comments = [mockCommentOne]; commentList.clickRow.subscribe((selectedComment: CommentModel) => { expect(selectedComment.id).toEqual(1); @@ -75,28 +72,7 @@ describe('CommentListComponent', () => { fixture.detectChanges(); fixture.whenStable().then(() => { - const comment = fixture.debugElement.query(By.css('#adf-comment-1')); - comment.triggerEventHandler('click', null); - }); - })); - - it('should deselect the previous selected comment when a new one is clicked', fakeAsync(() => { - mockCommentOne.isSelected = true; - const commentOne = Object.assign({}, mockCommentOne); - const commentTwo = Object.assign({}, mockCommentTwo); - commentList.selectedComment = commentOne; - commentList.comments = [commentOne, commentTwo]; - - commentList.clickRow.subscribe(() => { - fixture.detectChanges(); - const commentSelectedList = fixture.nativeElement.querySelectorAll('.adf-is-selected'); - expect(commentSelectedList.length).toBe(1); - expect(commentSelectedList[0].textContent).toContain('2nd Test Comment'); - }); - - fixture.detectChanges(); - fixture.whenStable().then(() => { - const comment = fixture.debugElement.query(By.css('#adf-comment-2')); + const comment = fixture.debugElement.query(By.css('.adf-comment-list:first-child')); comment.triggerEventHandler('click', null); }); })); @@ -109,7 +85,7 @@ describe('CommentListComponent', () => { }); it('should show comment message when input is given', async () => { - commentList.comments = [Object.assign({}, mockCommentOne)]; + commentList.comments = [mockCommentOne]; fixture.detectChanges(); await fixture.whenStable(); @@ -121,19 +97,19 @@ describe('CommentListComponent', () => { }); it('should show comment user when input is given', async () => { - commentList.comments = [Object.assign({}, mockCommentOne)]; + commentList.comments = [mockCommentOne]; fixture.detectChanges(); await fixture.whenStable(); const elements = fixture.nativeElement.querySelectorAll('.adf-comment-user-name'); expect(elements.length).toBe(1); - expect(elements[0].innerText).toBe(mockCommentOne.createdBy.firstName + ' ' + mockCommentOne.createdBy.lastName); + expect(elements[0].innerText).toBe(mockCommentOne.userDisplayName); expect(fixture.nativeElement.querySelector('.adf-comment-user-name:empty')).toBeNull(); }); it('comment date time should start with few seconds ago when comment date is few seconds ago', async () => { - const commentFewSecond = Object.assign({}, mockCommentOne); + const commentFewSecond = new CommentModel(mockCommentOne); commentFewSecond.created = new Date(); commentList.comments = [commentFewSecond]; @@ -146,7 +122,7 @@ describe('CommentListComponent', () => { }); it('comment date time should start with Yesterday when comment date is yesterday', async () => { - const commentOld = Object.assign({}, mockCommentOne); + const commentOld = new CommentModel(mockCommentOne); commentOld.created = new Date((Date.now() - 24 * 3600 * 1000)); commentList.comments = [commentOld]; @@ -158,7 +134,7 @@ describe('CommentListComponent', () => { }); it('comment date time should not start with Today/Yesterday when comment date is before yesterday', async () => { - const commentOld = Object.assign({}, mockCommentOne); + const commentOld = new CommentModel(mockCommentOne); commentOld.created = new Date((Date.now() - 24 * 3600 * 1000 * 2)); commentList.comments = [commentOld]; @@ -171,14 +147,14 @@ describe('CommentListComponent', () => { }); it('should show user icon when input is given', async () => { - commentList.comments = [Object.assign({}, mockCommentOne)]; + commentList.comments = [mockCommentOne]; fixture.detectChanges(); await fixture.whenStable(); const elements = fixture.nativeElement.querySelectorAll('.adf-comment-img-container'); expect(elements.length).toBe(1); - expect(elements[0].innerText).toContain(commentList.getUserShortName(mockCommentOne.createdBy)); + expect(elements[0].innerText).toContain(mockCommentOne.userInitials); expect(fixture.nativeElement.querySelector('.adf-comment-img-container:empty')).toBeNull(); }); diff --git a/lib/core/src/lib/comments/comment-list/comment-list.component.ts b/lib/core/src/lib/comments/comment-list/comment-list.component.ts index 957f48639a9..e676cb55322 100644 --- a/lib/core/src/lib/comments/comment-list/comment-list.component.ts +++ b/lib/core/src/lib/comments/comment-list/comment-list.component.ts @@ -15,11 +15,9 @@ * limitations under the License. */ -import { Component, EventEmitter, Input, Output, ViewEncapsulation, OnInit, OnDestroy, Inject } from '@angular/core'; +import { Component, EventEmitter, Input, Output, ViewEncapsulation, inject } from '@angular/core'; import { CommentModel } from '../../models/comment.model'; -import { UserPreferencesService, UserPreferenceValues } from '../../common/services/user-preferences.service'; -import { Subject } from 'rxjs'; -import { takeUntil } from 'rxjs/operators'; +import { User } from '../../models/general-user.model'; import { CommentsService } from '../interfaces/comments-service.interface'; import { ADF_COMMENTS_SERVICE } from '../interfaces/comments.token'; @@ -29,8 +27,7 @@ import { ADF_COMMENTS_SERVICE } from '../interfaces/comments.token'; styleUrls: ['./comment-list.component.scss'], encapsulation: ViewEncapsulation.None }) - -export class CommentListComponent implements OnInit, OnDestroy { +export class CommentListComponent { /** The comments data used to populate the list. */ @Input() @@ -38,57 +35,15 @@ export class CommentListComponent implements OnInit, OnDestroy { /** Emitted when the user clicks on one of the comment rows. */ @Output() - clickRow: EventEmitter = new EventEmitter(); - - selectedComment: CommentModel; - currentLocale; - private onDestroy$ = new Subject(); - - constructor( - @Inject(ADF_COMMENTS_SERVICE) private commentsService: Partial, - public userPreferenceService: UserPreferencesService - ) { - } + clickRow = new EventEmitter(); - ngOnInit() { - this.userPreferenceService - .select(UserPreferenceValues.Locale) - .pipe(takeUntil(this.onDestroy$)) - .subscribe(locale => this.currentLocale = locale); - } - - ngOnDestroy() { - this.onDestroy$.next(true); - this.onDestroy$.complete(); - } + private commentsService = inject(ADF_COMMENTS_SERVICE); selectComment(comment: CommentModel): void { - if (this.selectedComment) { - this.selectedComment.isSelected = false; - } - comment.isSelected = true; - this.selectedComment = comment; - this.clickRow.emit(this.selectedComment); - } - - getUserShortName(user: any): string { - let shortName = ''; - if (user) { - if (user.firstName) { - shortName = user.firstName[0].toUpperCase(); - } - if (user.lastName) { - shortName += user.lastName[0].toUpperCase(); - } - } - return shortName; - } - - isPictureDefined(user: any): boolean { - return user.pictureId || user.avatarId; + this.clickRow.emit(comment); } - getUserImage(user: any): string { + getUserImage(user: User): string { return this.commentsService.getUserImage(user); } } diff --git a/lib/core/src/lib/comments/comment-list/mocks/comment-list.service.mock.ts b/lib/core/src/lib/comments/comment-list/mocks/comment-list.service.mock.ts index 2617824cbde..c32c4fe8870 100644 --- a/lib/core/src/lib/comments/comment-list/mocks/comment-list.service.mock.ts +++ b/lib/core/src/lib/comments/comment-list/mocks/comment-list.service.mock.ts @@ -18,9 +18,6 @@ import { CommentsService } from '../../interfaces/comments-service.interface'; export class CommentListServiceMock implements Partial { - - constructor() {} - getUserImage(_user: any): string { return 'mock-user-image-path'; } diff --git a/lib/core/src/lib/comments/comments.component.html b/lib/core/src/lib/comments/comments.component.html index 1bdf00c96d4..45664daa7c4 100644 --- a/lib/core/src/lib/comments/comments.component.html +++ b/lib/core/src/lib/comments/comments.component.html @@ -1,10 +1,10 @@
-
+
{{'COMMENTS.HEADER' | translate: { count: comments?.length } }}
-
- -