Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { KbqTopBarModule } from '@koobiq/components/top-bar';
navigating away from the current view.
</p>

<button kbq-button kbqPopover [kbqPopoverContent]="popoverContent" [hideIfNotInViewPort]="false">
<button kbq-button kbqPopover [kbqPopoverContent]="popoverContent" [kbqPopoverHideIfNotInViewPort]="false">
Open popover
</button>

Expand Down
20 changes: 20 additions & 0 deletions docs/guides/migration.en.md
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,26 @@ Most of them report rather than rewrite: what replaces a removed member or a sig
ng g @koobiq/components:<schematic-name> --project <your project>
```

#### Popover

Hover mode was broken end to end by a dead expression. `this.leaveDelay ?? 500` looks like a default, but the base class sets the field to `0`, and `0 ?? 500` is `0` — so the panel closed before the pointer could cross the 8px gap to it, the documented interactive content was unreachable even for pointer users, and the auto-hide watchdog spun as an `interval(0)` for as long as the panel stayed open.

The delay is derived from the trigger now, and `kbqLeaveDelay` is a write-only input that records having been bound. Bound in the template, the bound value stands; not bound, the `trigger` setter re-derives the delay on every change, so a popover switched to `hover` later gets the hover default instead of the `0` it was born with.

A programmatic `trigger.leaveDelay = 500` records nothing, so the next write to `trigger` overwrites it. That is the one change here with no compile error behind it.

| Pattern | Manual migration |
| ----------------- | ------------------------------------------------------------------------------------- |
| `.leaveDelay = …` | Bind `[kbqLeaveDelay]`, or drop it and take the hover default — it is long enough now |
| `.onConfirm = …` | `onConfirm` is readonly; subscribe instead of replacing it |
| `placementChange` | Emits `string` instead of `any`; only a payload assigned to a non-string breaks |

The confirm popover no longer hardcodes its Russian defaults: «Вы уверены, что хотите продолжить?» and «Да» come from the locale now, so a non-RU application renders translated text where it used to render Russian.

Two fixes with nothing to migrate: the trigger subscribed to the global `ScrollDispatcher` with no teardown in the _default_ configuration, and that subscription is bounded now — a host that worked around the leak by destroying triggers eagerly can stop. And `KbqPopoverTrigger` can be imported standalone, because the scroll-strategy provider is no longer NgModule-only.

Reported by `popover-leave-delay`.

#### Search expandable

Step 4 already renames the `kbq-filter-search` element to `kbq-search-expandable`. That rewrite only ever touched the tag, so the inputs of the removed `KbqFilterBarSearch` survived as attributes the new component does not have — silently, because an unknown attribute on a component is not an error. `v20-upgrade` renames them too now:
Expand Down
20 changes: 20 additions & 0 deletions docs/guides/migration.ru.md
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,26 @@ ng update @koobiq/components@20
ng g @koobiq/components:<schematic-name> --project <your project>
```

#### Popover

Режим по наведению был сломан целиком из-за мёртвого выражения. `this.leaveDelay ?? 500` выглядит как значение по умолчанию, но базовый класс присваивает полю `0`, а `0 ?? 500` — это `0`. Панель закрывалась раньше, чем указатель успевал пересечь зазор в 8px до неё, задокументированное интерактивное содержимое было недостижимо даже для мыши, а таймер автоматического скрытия крутился как `interval(0)` всё время, пока панель открыта.

Теперь задержка выводится из триггера, а `kbqLeaveDelay` — вход только на запись, который запоминает сам факт привязки. Если привязка есть в шаблоне, действует привязанное значение; если нет — сеттер `trigger` пересчитывает задержку при каждом изменении, поэтому поповер, переключённый на `hover` позже, получает значение по умолчанию для наведения, а не `0`, с которым он был создан.

Программное присваивание `trigger.leaveDelay = 500` ничего не запоминает, поэтому следующая запись в `trigger` его перезапишет. Это единственное изменение здесь, за которым не стоит ошибки компиляции.

| Что было | Что делать вручную |
| ----------------- | --------------------------------------------------------------------------------------- |
| `.leaveDelay = …` | Привязать `[kbqLeaveDelay]` либо убрать вовсе — значения по умолчанию теперь достаточно |
| `.onConfirm = …` | `onConfirm` доступен только для чтения: подписывайтесь вместо замены |
| `placementChange` | Отдаёт `string` вместо `any`; ломается только присваивание значения не в строку |

Поповер подтверждения больше не зашивает русские значения по умолчанию: «Вы уверены, что хотите продолжить?» и «Да» берутся из локали, поэтому не-русское приложение получит переведённый текст там, где раньше был русский.

Два исправления, для которых мигрировать нечего: в _конфигурации по умолчанию_ триггер подписывался на глобальный `ScrollDispatcher` без отписки — теперь подписка ограничена по времени жизни, и хост, обходивший утечку принудительным уничтожением триггеров, может это убрать. И `KbqPopoverTrigger` можно импортировать как standalone: провайдер стратегии скролла больше не живёт только в NgModule.

Сообщает `popover-leave-delay`.

#### Search expandable

Шаг 4 уже переименовывает элемент `kbq-filter-search` в `kbq-search-expandable`. Но та замена трогала только тег, поэтому входы удалённого `KbqFilterBarSearch` оставались в разметке атрибутами, которых у нового компонента нет, — и молча, потому что неизвестный атрибут на компоненте не является ошибкой. Теперь `v20-upgrade` переименовывает и их:
Expand Down
6 changes: 3 additions & 3 deletions packages/components-dev/popover/template.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
kbqPopover
[kbqPopoverContent]="'Popover content'"
[kbqTooltip]="'Backdrop trigger tooltip'"
[hasBackdrop]="true"
[kbqPopoverHasBackdrop]="true"
>
Popover with backdrop
</button>
Expand Down Expand Up @@ -334,7 +334,7 @@ <h3>Configuration:</h3>
<button
kbq-button
kbqPopover
[hasCloseButton]="isClosable"
[kbqPopoverHasCloseButton]="isClosable"
[kbqPopoverClass]="'dev-popover-485'"
[kbqPopoverContent]="customContent"
[kbqPopoverFooter]="customFooter"
Expand Down Expand Up @@ -452,7 +452,7 @@ <h3>Configuration:</h3>
<button
kbq-button
kbqPopover
[hasCloseButton]="true"
[kbqPopoverHasCloseButton]="true"
[kbqPopoverClass]="'dev-popover-485'"
[kbqPopoverContent]="customContent"
[kbqPopoverFooter]="customFooter"
Expand Down
4 changes: 4 additions & 0 deletions packages/components/core/locales/en-US.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ export const enUSLocaleData = {
sitesHeader: 'Other sites',
clearSearch: 'Clear search'
},
popoverConfirm: {
confirmText: 'Are you sure you want to continue?',
confirmButtonText: 'Yes'
},
timeRange: {
title: {
for: 'for',
Expand Down
4 changes: 4 additions & 0 deletions packages/components/core/locales/es-LA.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ export const esLALocaleData = {
sitesHeader: 'Otros sitios',
clearSearch: 'Borrar la búsqueda'
},
popoverConfirm: {
confirmText: '¿Está seguro de que desea continuar?',
confirmButtonText: 'Sí'
},
timeRange: {
title: {
for: 'para',
Expand Down
1 change: 1 addition & 0 deletions packages/components/core/locales/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export * from './a11y';
export * from './configuration';
export * from './en-US';
export * from './es-LA';
export * from './popover-confirm';
export * from './pt-BR';
export * from './ru-RU';
export * from './select';
Expand Down
86 changes: 86 additions & 0 deletions packages/components/core/locales/popover-confirm.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { TestBed } from '@angular/core/testing';
import { BehaviorSubject } from 'rxjs';
import { enUSLocaleData } from './en-US';
import { esLALocaleData } from './es-LA';
import { KBQ_LOCALE_SERVICE, KbqLocaleService } from './locale-service';
import {
kbqInjectPopoverConfirmLocaleConfiguration,
kbqPopoverConfirmLocaleConfigurationProvider
} from './popover-confirm';
import { ptBRLocaleData } from './pt-BR';
import { ruRULocaleData } from './ru-RU';
import { tkTMLocaleData } from './tk-TM';
import { KbqPopoverConfirmLocaleConfiguration } from './types';

// The section is read through the shared `kbqInjectLocaleConfiguration`, whose behaviour is covered once in
// `a11y.spec.ts`. What is left to pin here is that the confirmation popover is wired to that helper with its
// own section name and token.
describe('kbqInjectPopoverConfirmLocaleConfiguration', () => {
const inject = () => TestBed.runInInjectionContext(kbqInjectPopoverConfirmLocaleConfiguration);

it('should fall back to the default locale when no locale service is provided', () => {
TestBed.configureTestingModule({});

expect(inject()()).toBe(ruRULocaleData.popoverConfirm);
});

it('should follow the locale service', () => {
TestBed.configureTestingModule({
providers: [{ provide: KBQ_LOCALE_SERVICE, useClass: KbqLocaleService }]
});

const configuration = inject();

expect(configuration().confirmText).toBe(ruRULocaleData.popoverConfirm.confirmText);

TestBed.inject(KBQ_LOCALE_SERVICE).setLocale('en-US');

expect(configuration().confirmText).toBe(enUSLocaleData.popoverConfirm.confirmText);
});

it('should apply the override on top of the active locale', () => {
TestBed.configureTestingModule({
providers: [
{ provide: KBQ_LOCALE_SERVICE, useClass: KbqLocaleService },
kbqPopoverConfirmLocaleConfigurationProvider({ confirmButtonText: 'Delete' })
]
});

const configuration = inject();

expect(configuration().confirmButtonText).toBe('Delete');

TestBed.inject(KBQ_LOCALE_SERVICE).setLocale('en-US');

// The overridden caption stays pinned; the rest of the section follows the locale.
expect(configuration().confirmButtonText).toBe('Delete');
expect(configuration().confirmText).toBe(enUSLocaleData.popoverConfirm.confirmText);
});

it('should fall back when the locale service hands back no section at all', () => {
// `KbqLocaleService` itself always completes a section, but applications routinely provide a stand-in
// under `KBQ_LOCALE_SERVICE` in their own tests — that one is free to return nothing.
const stub = { changes: new BehaviorSubject('custom'), getParams: () => undefined };

TestBed.configureTestingModule({ providers: [{ provide: KBQ_LOCALE_SERVICE, useValue: stub }] });

expect(inject()()).toBe(ruRULocaleData.popoverConfirm);
});
});

describe('popoverConfirm locale data', () => {
// A missing string leaves the confirmation popover rendering `undefined` in that locale only, which no
// component test would catch — the section is asserted complete for every shipped locale instead.
const locales: [string, KbqPopoverConfirmLocaleConfiguration][] = [
['en-US', enUSLocaleData.popoverConfirm],
['es-LA', esLALocaleData.popoverConfirm],
['pt-BR', ptBRLocaleData.popoverConfirm],
['ru-RU', ruRULocaleData.popoverConfirm],
['tk-TM', tkTMLocaleData.popoverConfirm]
];

it.each(locales)('should provide every string for %s', (_, data) => {
expect(Object.keys(data).sort()).toEqual(Object.keys(ruRULocaleData.popoverConfirm).sort());
Object.values(data).forEach((text) => expect(text.trim()).not.toBe(''));
});
});
32 changes: 32 additions & 0 deletions packages/components/core/locales/popover-confirm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { InjectionToken, Provider, Signal } from '@angular/core';
import { KbqDeepPartial } from '../utils';
import { kbqInjectLocaleConfiguration, kbqLocaleConfigurationOverrideProvider } from './configuration';
import { ruRULocaleData } from './ru-RU';
import { KbqPopoverConfirmLocaleConfiguration } from './types';

/** Localization configuration provider for the default strings of a confirmation popover. */
export const KBQ_POPOVER_CONFIRM_LOCALE_CONFIGURATION = new InjectionToken<KbqPopoverConfirmLocaleConfiguration>(
'KbqPopoverConfirmLocaleConfiguration',
{
factory: () => ruRULocaleData.popoverConfirm
}
);

/**
* Utility provider. Only the strings you pass are overridden; the rest keep following the active locale.
*
* @see KBQ_POPOVER_CONFIRM_LOCALE_CONFIGURATION
*/
export const kbqPopoverConfirmLocaleConfigurationProvider = (
configuration: KbqDeepPartial<KbqPopoverConfirmLocaleConfiguration>
): Provider => kbqLocaleConfigurationOverrideProvider('popoverConfirm', configuration);

/**
* Injection function that creates a reactive locale configuration signal with the default strings of a
* confirmation popover.
*
* @docs-private
*/
export function kbqInjectPopoverConfirmLocaleConfiguration(): Signal<KbqPopoverConfirmLocaleConfiguration> {
return kbqInjectLocaleConfiguration('popoverConfirm', KBQ_POPOVER_CONFIRM_LOCALE_CONFIGURATION);
}
4 changes: 4 additions & 0 deletions packages/components/core/locales/pt-BR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ export const ptBRLocaleData = {
sitesHeader: 'Outros sites',
clearSearch: 'Limpar a pesquisa'
},
popoverConfirm: {
confirmText: 'Tem certeza de que deseja continuar?',
confirmButtonText: 'Sim'
},
timeRange: {
title: {
for: 'para',
Expand Down
4 changes: 4 additions & 0 deletions packages/components/core/locales/ru-RU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ export const ruRULocaleData = {
sitesHeader: 'Другие площадки',
clearSearch: 'Очистить поиск'
},
popoverConfirm: {
confirmText: 'Вы уверены, что хотите продолжить?',
confirmButtonText: 'Да'
},
timeRange: {
title: {
for: 'за',
Expand Down
4 changes: 4 additions & 0 deletions packages/components/core/locales/tk-TM.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ export const tkTMLocaleData = {
sitesHeader: 'Beýleki saýtlar',
clearSearch: 'Gözlegi arassala'
},
popoverConfirm: {
confirmText: 'Dowam etmek isleýändigiňize ynanýarsyňyzmy?',
confirmButtonText: 'Hawa'
},
timeRange: {
title: {
for: 'soňky',
Expand Down
14 changes: 14 additions & 0 deletions packages/components/core/locales/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,19 @@ export type KbqSelectLocaleConfiguration = {
selectAll: string;
};

/**
* Locale configuration for the `[kbqPopoverConfirm]` variant of `KbqPopoverModule`.
*
* Unlike {@link KbqA11yLocaleConfiguration} these strings are displayed: they are the default question and
* the default confirm-button caption of a confirmation popover.
*/
export type KbqPopoverConfirmLocaleConfiguration = {
/** Question rendered in the body of the confirmation popover. */
confirmText: string;
/** Caption of the button that confirms the action. */
confirmButtonText: string;
};

/** Locale configuration for `KbqAppSwitcherModule`: the strings rendered by the app-switcher popup. */
export type KbqAppSwitcherLocaleConfiguration = {
/** Placeholder and accessible name of the search field. */
Expand Down Expand Up @@ -361,6 +374,7 @@ export interface KbqLocaleStringsData {
appSwitcher: KbqAppSwitcherLocaleConfiguration;
timeRange: KbqTimeRangeLocaleConfiguration;
notificationCenter: KbqNotificationCenterLocaleConfiguration;
popoverConfirm: KbqPopoverConfirmLocaleConfiguration;
}

/** The number and size formatting rules of a locale — the shape of `ruRUFormattersData` and its siblings. */
Expand Down
3 changes: 2 additions & 1 deletion packages/components/popover/e2e.playwright-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ test.describe('KbqPopoverModule', () => {
});

await page.mouse.move(gapPoint.x, gapPoint.y);
// Longer than the hover leaveDelay (500ms): if the gap were a dead zone the popover would have closed.
// What keeps the popover open is the pane covering the gap, so the pointer is still on the panel;
// the wait only has to outlast the hover leaveDelay (500 ms) for a dead zone to have closed it.
await page.waitForTimeout(800);

await expect(container).toBeVisible();
Expand Down
7 changes: 6 additions & 1 deletion packages/components/popover/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ import { KbqToolTipModule } from '@koobiq/components/tooltip';
</div>
</ng-template>

<div kbqPopover></div>
<!--
Exists only to satisfy the required KbqPopoverTrigger query below: the panels are instantiated
directly. A button rather than a div, because a popover trigger must be focusable, and hidden
so it stays out of the screenshot.
-->
<button type="button" kbqPopover hidden></button>
<div class="layout-row" style="gap: 16px">
<kbq-popover-component #popoverSmall />
<kbq-popover-component #popoverMedium />
Expand Down
21 changes: 18 additions & 3 deletions packages/components/popover/popover-animations.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,24 @@
import { animate, AnimationTriggerMetadata, state, style, transition, trigger } from '@angular/animations';
import { KbqAnimationCurves, KbqAnimationDurations } from '@koobiq/components/core';

/** Duration of the opening animation. */
const enterDuration = '120ms';

/** Duration of the closing animation. */
const exitDuration = KbqAnimationDurations.Rapid;

/**
* Animation that transitions a popover in and out.
*
* Motion is opt-out rather than unconditional: `KbqPopoverComponent` binds `[@.disabled]` to the user's
* `prefers-reduced-motion` setting, so the same trigger renders instantly for those users.
*
* @docs-private
*/
export const kbqPopoverAnimations: {
readonly popoverState: AnimationTriggerMetadata;
} = {
/** Animation that transitions a tooltip in and out. */
/** Animation that transitions a popover in and out. */
popoverState: trigger('state', [
state(
'initial',
Expand All @@ -15,13 +30,13 @@ export const kbqPopoverAnimations: {
transition(
'* => visible',
animate(
'120ms cubic-bezier(0, 0, 0.2, 1)',
`${enterDuration} ${KbqAnimationCurves.DecelerationCurve}`,
style({
opacity: 1,
transform: 'scale(1, 1)'
})
)
),
transition('* => hidden', animate('100ms linear', style({ opacity: 0 })))
transition('* => hidden', animate(`${exitDuration} linear`, style({ opacity: 0 })))
])
};
Loading