Skip to content

Commit

Permalink
chore(UVE): Toolbar Enhancements #30940 (#31067)
Browse files Browse the repository at this point in the history
## Tasks

- [x] Be sure that all buttons has tooltips and position should be
bottom
- [x]  Change the Icon for the copy url
- [x]  unify color of borders, in preview, edit and divider
- [x]  fix double border
- [x]  Issue with info display showing on user locking the page

## Screenshots



https://github.com/user-attachments/assets/db6a0c27-79b2-4e8f-a699-ad0fb38c5756
  • Loading branch information
zJaaal authored Jan 8, 2025
1 parent c8c971b commit eba9158
Show file tree
Hide file tree
Showing 14 changed files with 226 additions and 163 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ p-toolbar::ng-deep {
color: $toolbar-color;
height: $toolbar-height;
padding: 0 $spacing-3 0 0;
border: 0;
border-bottom: solid 1px $color-palette-gray-300;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
@if ($options()?.actionIcon) {
@let options = $options();

@if (options?.actionIcon) {
<p-button
(onClick)="handleAction()"
[icon]="$options()?.actionIcon"
[icon]="options?.actionIcon"
styleClass="p-button-text p-button-sm p-button-rounded"
data-testId="info-action"></p-button>
}

<div class="info-container">
@if (!!$options()?.icon) {
<i [class]="$options()?.icon + ' info-container__icon'" data-testId="info-icon"></i>
@if (!!options?.icon) {
<i [class]="options?.icon + ' info-container__icon'" data-testId="info-icon"></i>
}
<span
[innerHTML]="$options()?.info.message | dm: $options()?.info.args"
[innerHTML]="options?.info.message | dm: options?.info.args"
data-testId="info-text"></span>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import {
createComponentFactory,
mockProvider
} from '@ngneat/spectator/jest';
import { patchState } from '@ngrx/signals';
import { of } from 'rxjs';

import { CommonModule } from '@angular/common';
import { HttpClientTestingModule } from '@angular/common/http/testing';
import { signal } from '@angular/core';
import { By } from '@angular/platform-browser';
import { ActivatedRoute, Router } from '@angular/router';

Expand All @@ -25,19 +25,15 @@ import {
DotWorkflowsActionsService
} from '@dotcms/data-access';
import { LoginService } from '@dotcms/dotcms-js';
import { DEFAULT_VARIANT_NAME } from '@dotcms/dotcms-models';
import {
DotExperimentsServiceMock,
DotLanguagesServiceMock,
DotLicenseServiceMock,
getRunningExperimentMock,
mockDotDevices
getRunningExperimentMock
} from '@dotcms/utils-testing';

import { DotEmaInfoDisplayComponent } from './dot-ema-info-display.component';

import { DotPageApiService } from '../../../../../services/dot-page-api.service';
import { DEFAULT_PERSONA } from '../../../../../shared/consts';
import { MOCK_RESPONSE_HEADLESS } from '../../../../../shared/mocks';
import { UVEStore } from '../../../../../store/dot-uve.store';

Expand All @@ -50,10 +46,16 @@ describe('DotEmaInfoDisplayComponent', () => {
component: DotEmaInfoDisplayComponent,
imports: [CommonModule, HttpClientTestingModule],
providers: [
UVEStore,
MessageService,
mockProvider(Router),
mockProvider(ActivatedRoute),
{
provide: UVEStore,
useValue: {
clearDeviceAndSocialMedia: jest.fn(),
experiment: signal(getRunningExperimentMock())
}
},
{
provide: DotWorkflowsActionsService,
useValue: {
Expand All @@ -66,7 +68,7 @@ describe('DotEmaInfoDisplayComponent', () => {
},
{
provide: DotExperimentsService,
useValue: DotExperimentsServiceMock
useValue: {}
},
{
provide: DotPageApiService,
Expand Down Expand Up @@ -99,71 +101,40 @@ describe('DotEmaInfoDisplayComponent', () => {
]
});

describe('device', () => {
beforeEach(() => {
spectator = createComponent();

store = spectator.inject(UVEStore);

store.loadPageAsset({
clientHost: 'http://localhost:3000',
url: 'index',
language_id: '1',
'com.dotmarketing.persona.id': DEFAULT_PERSONA.identifier
});
store.setDevice({ ...mockDotDevices[0], icon: 'test' });
});

it('should show name, sizes and icon of the selected device', () => {
spectator.detectChanges();
expect(spectator.query(byTestId('info-text')).textContent.trim()).toBe(
'iphone 200 x 100'
);
expect(spectator.query(byTestId('info-icon'))).not.toBeNull();
});

it('should call clearDeviceAndSocialMedia when action button is clicked', () => {
spectator.detectChanges();

const clearDeviceAndSocialMediaSpy = jest.spyOn(store, 'clearDeviceAndSocialMedia');

const infoAction = spectator.debugElement.query(By.css('[data-testId="info-action"]'));
beforeEach(() => {
spectator = createComponent();

spectator.triggerEventHandler(infoAction, 'onClick', store.$infoDisplayOptions());
store = spectator.inject(UVEStore);

expect(clearDeviceAndSocialMediaSpy).toHaveBeenCalled();
spectator.setInput('options', {
icon: `pi pi-facebook}`,
id: 'socialMedia',
info: {
message: `Viewing <b>facebook</b> social media preview`,
args: []
},
actionIcon: 'pi pi-times'
});
});

describe('socialMedia', () => {
beforeEach(() => {
spectator = createComponent();
store = spectator.inject(UVEStore);

store.loadPageAsset({
clientHost: 'http://localhost:3000',
url: 'index',
language_id: '1',
'com.dotmarketing.persona.id': DEFAULT_PERSONA.identifier
});

store.setSocialMedia('facebook');
spectator.detectChanges();
});

it('should text for current social media', () => {
describe('DOM', () => {
it('should show an icon and a text when passed through options', () => {
expect(spectator.query(byTestId('info-icon'))).not.toBeNull();
expect(spectator.query(byTestId('info-text')).textContent.trim()).toBe(
'Viewing facebook social media preview'
);
expect(spectator.query(byTestId('info-icon'))).not.toBeNull();
});

it('should have an actionIcon when provided', () => {
expect(spectator.query(byTestId('info-action'))).not.toBeNull();
});

it('should call clearDeviceAndSocialMedia when action button is clicked', () => {
const clearDeviceAndSocialMediaSpy = jest.spyOn(store, 'clearDeviceAndSocialMedia');

const infoAction = spectator.debugElement.query(By.css('[data-testId="info-action"]'));

spectator.triggerEventHandler(infoAction, 'onClick', store.$infoDisplayOptions());
spectator.triggerEventHandler(infoAction, 'onClick', {});

expect(clearDeviceAndSocialMediaSpy).toHaveBeenCalled();
});
Expand All @@ -172,50 +143,24 @@ describe('DotEmaInfoDisplayComponent', () => {
describe('variant', () => {
beforeEach(() => {
spectator = createComponent();

store = spectator.inject(UVEStore);
router = spectator.inject(Router);

store.loadPageAsset({
clientHost: 'http://localhost:3000',
url: 'index',
language_id: '1',
'com.dotmarketing.persona.id': DEFAULT_PERSONA.identifier,
variantId: '555-5555-5555-5555'
});

const currentExperiment = getRunningExperimentMock();

const variantID = currentExperiment.trafficProportion.variants.find(
(variant) => variant.name !== DEFAULT_VARIANT_NAME
).id;

patchState(store, {
pageAPIResponse: {
...MOCK_RESPONSE_HEADLESS,
viewAs: {
...MOCK_RESPONSE_HEADLESS.viewAs,
variantId: variantID
}
spectator.setInput('options', {
icon: 'pi pi-file-edit',
id: 'variant',
info: {
message: 'editpage.editing.variant',
args: ['Variant A']
},
experiment: currentExperiment
actionIcon: 'pi pi-arrow-left'
});
});
it('should show have text for variant', () => {
spectator.detectChanges();
expect(spectator.query(byTestId('info-text')).textContent.trim()).toBe(
'editpage.editing.variant'
);
expect(spectator.query(byTestId('info-icon'))).not.toBeNull();
});
it('should call router when action button is clicked', () => {
spectator.detectChanges();

const navigateSpy = jest.spyOn(router, 'navigate');

const infoAction = spectator.debugElement.query(By.css('[data-testId="info-action"]'));

spectator.triggerEventHandler(infoAction, 'onClick', store.$infoDisplayOptions());
spectator.triggerEventHandler(infoAction, 'onClick', {});

expect(navigateSpy).toHaveBeenCalledWith(
['/edit-page/experiments/', '456', '555-5555-5555-5555', 'configuration'],
Expand All @@ -226,53 +171,4 @@ describe('DotEmaInfoDisplayComponent', () => {
);
});
});

describe('edit permissions', () => {
beforeEach(() => {
spectator = createComponent();

store = spectator.inject(UVEStore);

patchState(store, {
canEditPage: false
});
});
it('should show label and icon for no permissions', () => {
spectator.detectChanges();
expect(spectator.query(byTestId('info-text')).textContent.trim()).toBe(
'editema.dont.have.edit.permission'
);
expect(spectator.query(byTestId('info-icon'))).not.toBeNull();
});
});

describe('edit permissions', () => {
beforeEach(() => {
spectator = createComponent();

store = spectator.inject(UVEStore);

patchState(store, {
pageAPIResponse: {
...MOCK_RESPONSE_HEADLESS,
page: {
...MOCK_RESPONSE_HEADLESS.page,
locked: true,
canLock: true,
lockedByName: 'John Doe'
}
}
});
});

it('should show label and icon for no permissions', () => {
spectator.detectChanges();

expect(spectator.query(byTestId('info-text')).textContent.trim()).toBe(
'editpage.locked-by'
);

expect(spectator.query(byTestId('info-icon'))).not.toBeNull();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { ChangeDetectionStrategy, Component, inject } from '@angular/core';
import { ChangeDetectionStrategy, Component, inject, input } from '@angular/core';
import { Router } from '@angular/router';

import { ButtonModule } from 'primeng/button';

import { DotMessagePipe } from '@dotcms/ui';

import { InfoOptions } from '../../../../../shared/models';
import { UVEStore } from '../../../../../store/dot-uve.store';

@Component({
Expand All @@ -19,7 +20,7 @@ export class DotEmaInfoDisplayComponent {
protected readonly uveStore = inject(UVEStore);
protected readonly router = inject(Router);

protected readonly $options = this.uveStore.$infoDisplayOptions;
$options = input<InfoOptions>(undefined, { alias: 'options' });

/**
* Handle the action based on the options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
[pTooltip]="'uve.preview.mode' | dm"
tooltipPosition="bottom"
icon="pi pi-eye"
[tooltipPosition]="'bottom'"
styleClass="p-button-text p-button-sm p-button-rounded"
data-testId="uve-toolbar-preview"
(click)="triggerPreviewMode()" />
Expand All @@ -67,15 +68,15 @@
<p-button
[pTooltip]="'editpage.header.copy' | dm"
tooltipPosition="bottom"
icon="pi pi-external-link"
icon="pi pi-copy"
styleClass="p-button-text p-button-sm p-button-rounded"
(cdkCopyToClipboardCopied)="triggerCopyToast()"
[cdkCopyToClipboard]="$toolbar().editor.copyUrl"
data-testId="uve-toolbar-copy-url" />

<a
title="Page API URL"
[pTooltip]="'uve.preview.page.api.url' | dm"
[pTooltip]="'uve.toolbar.page.api.url' | dm"
tooltipPosition="bottom"
pButton
label="API"
Expand All @@ -98,6 +99,6 @@
}
</ng-template>

@if ($infoDisplayProps()) {
<dot-ema-info-display data-testId="info-display" />
@if ($infoDisplayProps(); as options) {
<dot-ema-info-display data-testId="info-display" [options]="options" />
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
.uve-toolbar {
padding: 0 $spacing-4;
transition: padding 0.2s ease;
border-top: 1px solid transparent; // Avoid jump
}

.p-button.uve-toolbar-chips,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,10 @@ describe('DotUveToolbarComponent', () => {
it('should have attrs', () => {
expect(button.attributes).toEqual({
class: 'ng-star-inserted',
icon: 'pi pi-external-link',
icon: 'pi pi-copy',
'data-testId': 'uve-toolbar-copy-url',
'ng-reflect-style-class': 'p-button-text p-button-sm p-bu',
'ng-reflect-icon': 'pi pi-external-link',
'ng-reflect-icon': 'pi pi-copy',
'ng-reflect-text': 'http://localhost:3000/test-url',
'ng-reflect-tooltip-position': 'bottom',
tooltipPosition: 'bottom',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,5 @@
</p-toolbar>

@if ($toolbarProps().showInfoDisplay) {
<dot-ema-info-display data-testId="info-display" />
<dot-ema-info-display data-testId="info-display" [options]="$infoOptions()" />
}
Loading

0 comments on commit eba9158

Please sign in to comment.