Skip to content

feat: added href to card - #10660

Open
BF150 wants to merge 23 commits into
developfrom
feat/10123-linked-card
Open

feat: added href to card#10660
BF150 wants to merge 23 commits into
developfrom
feat/10123-linked-card

Conversation

@BF150

@BF150 BF150 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@BF150 BF150 self-assigned this Aug 10, 2026
@BF150 BF150 added the release:feature PR fügt ein neues Feature hinzu und wird unter 'New Features' im Release-Changelog gelistet. label Aug 10, 2026
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

🚀 MCP preview deployed to Vercel: https://kolibri-9apzfh4rm-public-ui-kolibri-mcp.vercel.app

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

🚀 Preview deployed to GitHub Pages: https://public-ui.github.io/kolibri/pr-10660/f7dc851/
Commit: f7dc851

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

📸 Visual Test Reports

Theme Report
theme-bwst Open report
theme-default Open report
theme-desy Open report
theme-ecl Open report
theme-kern Open report
test-tag-name-transformer Open report

Run: https://github.com/public-ui/kolibri/actions/runs/31581991199

@BF150
BF150 requested a review from deleonio August 11, 2026 06:33
@BF150

BF150 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@deleonio Ich bekomme die Click/Focus Decorator nicht zum laufen. Können wir da heute mal zusammen drauf schauen?

@deleonio

Copy link
Copy Markdown
Contributor

Hi @BF150 👋
Hier die Erkenntnisse zum PR — vorab direkt zu deiner Frage:

Warum die Click/Focus-Decoratoren nicht laufen 🔴

In shadow.tsx fehlt @Element() host. Die delegateFocus/delegateClick-Decoratoren lesen this.host und brechen stillschweigend ab, wenn es nicht existiert (element-interaction.ts: const host = self['host']; if (!host) return;). Die innere kol-card-wc mit @directFocus würde funktionieren, aber die öffentliche API kol-card.focus() / .click() ist aktuell ein No-Op.

Fix in shadow.tsx:

export class KolCard implements CardProps, ClickableElement, FocusableElement {
	@Element() protected readonly host?: HTMLKolCardElement; // ← ergänzen
	protected readonly ctaRef = createCtaRef<HTMLKolCardWcElement>();

Vergleich: link/shadow.tsx:39 macht genau das.

validateHref({ required: true }) vs. optionaler Prop 🟠

_href ist optional deklariert (_href?:) und im Render per this._href && … bedingt — Karten ohne Link sind der Normalfall. Trotzdem wird mit required: true validiert. watchValidator loggt dann bei jeder nicht-verlinkten Karte eine Dev-Warnung. Codebaseweit gibt es diese Kombi (optional + required) sonst nicht.

Fix: required: true weglassen → validateHref(this, value) (analog tree-item).

Vor Merge klären 🟡

1. Stretched-Link überdeckt Slot & Close-Button:
.kol-card__link::after { inset: 0 } legt sich als klickbare Fläche über die ganze Karte. Da der <a> nur das Heading umschließt, werden interaktive Slott-Elemente (Links/Buttons vom Konsumenten) und der Close-Button (_hasCloser=true) blockiert. → Verlinkte Karte + Closer ist derzeit funktional kaputt. Entweder dokumentieren ("verlinkte Karte ohne weitere Interaktionen") oder die pointer-events-Architektur überdenken (interaktive Kinder brauchen eigenen Stacking-Kontext).

2. ECL bricht das Konzept:
ecl-ec/mixins/card.scss setzt __link::after { display: none } → bei ECL ist nur das Heading klickbar, nicht die ganze Karte. Inkonsistent zu den anderen Themes. Bewusst? Dann bitte dokumentieren.

3. KolFocusOptions-Import prüfen:
Du nutzt KolFocusOptions in focus(), aber im Diff-Import-Block taucht es nicht auf. Konvention ist Named-Import aus '../../schema' (vgl. button/component.tsx:21). Falls es wirklich fehlt → TS-Fehler; am Code gegenprüfen.

Minor / Cleanup ⚪

  • desy/src/mixins/alert.scss ist nur eine Stylelint-Property-Neusortierung und hat nichts mit dem Feature zu tun → besser in einen separaten Commit auslagern.
  • linked.tsx: kopierter Kommentar "Ensure the popover is closed on initial render" stammt noch aus dem Popover-Sample — für die Karte falsch, bitte anpassen.
  • Optional: ein onClick-Callback in KoliBriCardEventCallbacks für SPA-Routing-Konsumenten (native Anker-Navigation funktioniert aber ohnehin).

Was super ist ✅

  • Direct/Delegate-Split korrekt gewählt (analog link/button/select).
  • Focus-Ring über ::after { @include focus-outline } = saubere, ganzflächige Indikation.
  • Vollständige Abdeckung: Schema, beide Komponenten-Dateien, Tests, Sample, alle Themes.

Sag Bescheid, wenn wir beim host-Fix gemeinsam drüberschauen wollen — das war der eigentliche Stolperstein. 🙂

@BF150

BF150 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Funktional stimmt jetzt alles.

zu 1. -> Für das Thema mit dem Close Button habe ich ein Ticket angelegt: #10672
zu 2. -> ECL hat laut Styleguide nur einen klickbare Headline, deswegen so umgesetzt.
"3. KolFocusOptions-Import prüfen:
Du nutzt KolFocusOptions in focus(), aber im Diff-Import-Block taucht es nicht auf. Konvention ist Named-Import aus '../../schema' (vgl. button/component.tsx:21). Falls es wirklich fehlt → TS-Fehler; am Code gegenprüfen." -> Was meint er damit?

Desy alert.scss war tatsächlich der Linter. Würde ich jetzt so lassen.

@BF150
BF150 marked this pull request as ready for review August 12, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:feature PR fügt ein neues Feature hinzu und wird unter 'New Features' im Release-Changelog gelistet.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

externe Themes: klickbare Card Vollflächig klickbare Card (nur Link über _href)

2 participants