Skip to content

Commit 78d41f7

Browse files
committed
refactor: replace KolPopover with PopoverFC in components
- Updated the KolPopoverButton and KolSplitButton components to use the new PopoverFC functional component. - Removed the legacy KolPopover component and its associated tests. - Introduced PopoverController and popoverPropsConfig for managing popover state and props. - Added align and show props for the new PopoverFC to control visibility and positioning. - Updated documentation to reflect changes in naming conventions and component architecture.
1 parent 3ad1e36 commit 78d41f7

File tree

16 files changed

+289
-427
lines changed

16 files changed

+289
-427
lines changed

packages/components/src/components/_skeleton/ARC42.md

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,22 @@ This modular layout is the backbone for the architectural patterns described in
7272
- **Stencil** is used for authoring web components with **shadow: true** only (Shadow DOM enabled for style isolation).
7373
- Components without Shadow DOM (`shadow: false`) are implemented as **Functional Components** instead of web components to avoid style conflicts and reduce maintainability burden.
7474
- Components must compile to framework-agnostic Custom Elements.
75-
- Public API properties use an underscored naming convention (e.g. `_name`) to separate external inputs from internal state.
75+
- **Public API properties (Web Component Props/Attributes) use an underscored naming convention** (e.g. `_name`) to separate external inputs from internal state. This convention applies **only at the Web Component boundary** (Props in `@Prop()` decorators). Internally, controller state, render props, and functional component parameters do **not** use underscore prefixes — they use clear, self-describing names (`visible`, `count`, `label`). The underscore signals "this is a managed property exposed to HTML consumers," not "this is internal."
7676
- Documentation and code follow the `KoliBri` casing and repository conventions.
7777

78+
### Naming Conventions Across Layers
79+
80+
The underscore naming convention is **scoped to the Web Component public API boundary**. Here's how naming changes across layers:
81+
82+
| Layer | Example | Purpose |
83+
| --------------------------------------- | ----------------------------------- | ------------------------------------------------------------------ |
84+
| **Web Component (Public API)** | `_name`, `_visible`, `_label` | Signals managed properties exposed to HTML consumers via `@Prop()` |
85+
| **Controller (Internal State)** | `name`, `visible`, `label`, `count` | Clear, self-describing state names — no underscore prefix needed |
86+
| **Functional Component (Render Props)** | `visible`, `align`, `count` | Parameters passed from controller — no underscore prefix |
87+
| **Schema Helpers** | `nameProp`, `visibleProp` | Prop definitions for validation and normalization |
88+
89+
**Rule of thumb**: Use underscore (`_`) **only** where consumers interact with the component via HTML (Web Component `@Prop()` decorators). Everywhere else — controllers, state, derived values, render props — use natural, descriptive names.
90+
7891
## 3. Context and Scope
7992

8093
The skeleton lives inside `packages/components` and does not depend on runtime frameworks. External consumers interact through HTML attributes or DOM APIs.
@@ -85,7 +98,7 @@ The skeleton lives inside `packages/components` and does not depend on runtime f
8598
- **Encapsulation**: Components maintain consistent appearance regardless of host environment
8699
- **Maintainability**: Clear boundaries prevent unintended style interactions
87100

88-
Components that historically did not use Shadow DOM (`shadow: false`) are being migrated to **Functional Components** instead, which can be composed into other components without Shadow DOM overhead while maintaining clean architectural separation.
101+
**Shadow DOM is mandatory for all KoliBri Web Components.** Components with `shadow: false` (historically suffixed `-wc`) are considered legacy and will be fully replaced and removed. The migration target is the Skeleton Pattern: each such component is rewritten as a proper Shadow DOM Web Component paired with a Functional Component for internal composition. No new `shadow: false` components will be introduced. The `-wc` variants are not a supported architecture going forward.
89102

90103
```mermaid
91104
flowchart LR

packages/components/src/components/_skeleton/TODO_PROP_ENFORCEMENT.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,16 @@ Wenn Props über `PropsConfigShape` + `ApiFromConfig` in `api.tsx` definiert wer
66

77
Das **Prop-Dreieck** besteht aus drei Teilen — nur einer davon ist type-erzwungen:
88

9-
| Teil | Type-erzwungen? | Was passiert bei Fehlen |
10-
|------|----------------|------------------------|
11-
| Felddeklaration `_name!: string` | **Ja** — via `ComponentProps<T>` in `WebComponentInterface` | compile error |
12-
| `@Prop()` Decorator | **Nein** | kein Attribut-Binding, silent runtime bug |
13-
| `@Watch('_name')` Decorator | **Nein** (`watchName()` Methode wird erzwungen, aber nicht der Decorator) | Stencil ruft Methode nie auf, silent runtime bug |
14-
| Forwarding in `componentWillLoad()` | **Nein** | Prop-Wert kommt nie im Controller an |
9+
| Teil | Type-erzwungen? | Was passiert bei Fehlen |
10+
| ----------------------------------- | ------------------------------------------------------------------------- | ------------------------------------------------ |
11+
| Felddeklaration `_name!: string` | **Ja** — via `ComponentProps<T>` in `WebComponentInterface` | compile error |
12+
| `@Prop()` Decorator | **Nein** | kein Attribut-Binding, silent runtime bug |
13+
| `@Watch('_name')` Decorator | **Nein** (`watchName()` Methode wird erzwungen, aber nicht der Decorator) | Stencil ruft Methode nie auf, silent runtime bug |
14+
| Forwarding in `componentWillLoad()` | **Nein** | Prop-Wert kommt nie im Controller an |
1515

1616
## Aktueller Stand
1717

18-
`implements WebComponentInterface<SkeletonApi>` ist korrekt und schon vorhanden — es deckt alles ab, was TypeScript abdecken *kann*. Die Lücke ist Stencil-spezifisch: Stencil-Decorators sind reine Metadaten und für das TypeScript-Typsystem unsichtbar.
18+
`implements WebComponentInterface<SkeletonApi>` ist korrekt und schon vorhanden — es deckt alles ab, was TypeScript abdecken _kann_. Die Lücke ist Stencil-spezifisch: Stencil-Decorators sind reine Metadaten und für das TypeScript-Typsystem unsichtbar.
1919

2020
## Lösungsoptionen
2121

packages/components/src/components/component-list.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import { KolModal } from './modal/shadow';
3636
import { KolNav } from './nav/shadow';
3737
import { KolPagination } from './pagination/shadow';
3838
import { KolPopoverButton } from './popover-button/shadow';
39-
import { KolPopover } from './popover/component';
4039
import { KolProgress } from './progress/component';
4140
import { KolQuote } from './quote/component';
4241
import { KolSelect } from './select/shadow';
@@ -93,7 +92,6 @@ export const COMPONENTS = [
9392
KolModal,
9493
KolNav,
9594
KolPagination,
96-
KolPopover,
9795
KolProgress,
9896
KolPopoverButton,
9997
KolQuote,

packages/components/src/components/popover-button/component.tsx

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { autoUpdate } from '@floating-ui/dom';
21
import type { JSX } from '@stencil/core';
32
import { Component, h, Method, Prop, State, Watch } from '@stencil/core';
43
import { KolButtonWcTag } from '../../core/component-names';
4+
import { PopoverFC } from '../../internal/functional-components/popover/component';
55
import type {
66
AccessKeyPropType,
77
AriaDescriptionPropType,
@@ -21,7 +21,6 @@ import type {
2121
} from '../../schema';
2222
import { validateInline, validatePopoverAlign } from '../../schema';
2323
import type { PopoverButtonProps, PopoverButtonStates } from '../../schema/components/popover-button';
24-
import { alignFloatingElements } from '../../utils/align-floating-elements';
2524
import clsx from '../../utils/clsx';
2625

2726
/**
@@ -36,7 +35,8 @@ import clsx from '../../utils/clsx';
3635
export class KolPopoverButton implements PopoverButtonProps {
3736
private refButton?: HTMLKolButtonWcElement;
3837
private refPopover?: HTMLDivElement;
39-
private cleanupAutoPositioning?: () => void;
38+
private handleToggleBound = this.handleToggle.bind(this);
39+
private handleBeforeToggleBound = this.handleBeforeToggle.bind(this);
4040
private on: ButtonCallbacksPropType<StencilUnknown> = {
4141
onClick: this.handleButtonClick.bind(this),
4242
};
@@ -83,39 +83,11 @@ export class KolPopoverButton implements PopoverButtonProps {
8383
// Reset the flag after the event loop tick.
8484
this.justClosed = false;
8585
}, 10); // timeout of 0 should be sufficient but doesn't work in Safari Mobile (needs further investigation).
86-
} else {
87-
if (this.refPopover) {
88-
/**
89-
* Avoid "flicker" by hiding the element until the position is set in the `toggle` event handler. `alignFloatingElements` is responsible for setting the visibility back to 'visible'.
90-
*/
91-
this.refPopover.style.visibility = 'hidden';
92-
}
93-
}
94-
}
95-
96-
private alignPopover() {
97-
if (this.refPopover && this.refButton) {
98-
void alignFloatingElements({
99-
align: this.state._popoverAlign,
100-
floatingElement: this.refPopover,
101-
referenceElement: this.refButton,
102-
});
10386
}
10487
}
10588

10689
private handleToggle(event: Event) {
10790
this.popoverOpen = (event as ToggleEvent).newState === 'open';
108-
109-
if (this.popoverOpen) {
110-
if (this.refPopover && this.refButton) {
111-
this.cleanupAutoPositioning = autoUpdate(this.refButton, this.refPopover, () => {
112-
this.alignPopover();
113-
});
114-
}
115-
} else if (this.cleanupAutoPositioning) {
116-
this.cleanupAutoPositioning();
117-
this.cleanupAutoPositioning = undefined;
118-
}
11991
}
12092

12193
private handleButtonClick() {
@@ -125,15 +97,22 @@ export class KolPopoverButton implements PopoverButtonProps {
12597
}
12698
}
12799

100+
private handleEscape = (event: KeyboardEvent) => {
101+
if (event.key === 'Escape') {
102+
this.refPopover?.hidePopover();
103+
}
104+
};
105+
128106
public componentDidRender() {
129-
this.refPopover?.addEventListener('toggle', this.handleToggle.bind(this));
130-
this.refPopover?.addEventListener('beforetoggle', this.handleBeforeToggle.bind(this));
107+
this.refPopover?.addEventListener('toggle', this.handleToggleBound);
108+
this.refPopover?.addEventListener('beforetoggle', this.handleBeforeToggleBound);
109+
this.refPopover?.addEventListener('keydown', this.handleEscape);
131110
}
132111

133112
public disconnectedCallback() {
134-
this.refPopover?.removeEventListener('toggle', this.handleToggle.bind(this));
135-
this.refPopover?.removeEventListener('beforetoggle', this.handleBeforeToggle.bind(this));
136-
this.cleanupAutoPositioning?.();
113+
this.refPopover?.removeEventListener('toggle', this.handleToggleBound);
114+
this.refPopover?.removeEventListener('beforetoggle', this.handleBeforeToggleBound);
115+
this.refPopover?.removeEventListener('keydown', this.handleEscape);
137116
}
138117

139118
public render(): JSX.Element {
@@ -174,9 +153,18 @@ export class KolPopoverButton implements PopoverButtonProps {
174153
<slot name="expert" slot="expert"></slot>
175154
</KolButtonWcTag>
176155

177-
<div ref={(element) => (this.refPopover = element)} data-testid="popover-content" popover="auto" id="popover" class="kol-popover-button__popover">
156+
<PopoverFC
157+
align={this.state._popoverAlign || 'bottom'}
158+
show={this.popoverOpen}
159+
visible={this.popoverOpen}
160+
refPopoverElement={(el) => (this.refPopover = el)}
161+
refArrowElement={() => {}}
162+
data-testid="popover-content"
163+
id="popover"
164+
class="kol-popover-button__popover"
165+
>
178166
<slot />
179-
</div>
167+
</PopoverFC>
180168
</div>
181169
);
182170
}

0 commit comments

Comments
 (0)