Skip to content

7991 - Badge: style-Attribute in Komponente durch css-Styles ersetzen#8001

Closed
aTTiCuZ-CodingPage wants to merge 2 commits intopublic-ui:developfrom
aTTiCuZ-CodingPage:7991-Badge-style-Attribute-in-Komponente-durch-css-ersetzen
Closed

7991 - Badge: style-Attribute in Komponente durch css-Styles ersetzen#8001
aTTiCuZ-CodingPage wants to merge 2 commits intopublic-ui:developfrom
aTTiCuZ-CodingPage:7991-Badge-style-Attribute-in-Komponente-durch-css-ersetzen

Conversation

@aTTiCuZ-CodingPage
Copy link
Copy Markdown
Contributor

Refs: #7991

Refactoring der Farbvergabe in KolBadge-Komponente

Ziel des PRs:

Entfernung von Inline-Styles für background-color und color innerhalb der KolBadge-Komponente zugunsten von CSS-Variablen und SCSS-Defaults. Dadurch wird das Styling konsistenter, wartbarer und besser konfigurierbar durch externe Stylesheets.

Änderungen im Detail

  • Entfernung von style-Attributen in der JSX-Render-Funktion.
  • bgColorStr und colorStr Variablen entfernt, da Styling nun über SCSS gesteuert wird.
  • Einführung des styleVars-State-Objekts für dynamisches Setzen von CSS-Variablen:
    • --kol-badge-bg-color
    • --kol-badge-text-color
  • Anpassung der handleColorChange()-Methode zur Nutzung von styleVars.
  • Neue Fallback-Logik: Wenn _color nicht gesetzt ist, greifen die in der style.scss definierten Defaults.
  • SCSS-Erweiterung: Standardfarben wurden mit var(--kol-badge-bg-color, #000) und var(--kol-badge-text-color, #959595) ergänzt.

Begründung

  • Trennung von Struktur (JSX) und Darstellung (SCSS).
  • Einheitliche Farbsteuerung über CSS-Variablen.
  • Bessere Wartbarkeit und Konsistenz innerhalb der Kolibri-Komponentenfamilie.

Tests & visuelles Verhalten

  • Die Komponente wurde in den Sample-Tests visuell geprüft.
  • Das Verhalten bleibt unverändert bei gesetztem _color.
  • Standard-Rendering funktioniert korrekt ohne _color-Angabe (Fallback auf SCSS-Farben).

@anicyne anicyne linked an issue Jul 8, 2025 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@sdvg sdvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich finde den Ansatz gut 👍

@Watch('_color')
public validateColor(value?: Stringified<PropColor>): void {
// Fallback auf SCSS-Styles, wenn _color nicht gesetzt ist
if (!value) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Können wir hier nicht weiter mit der defaultValue-Eigenschaft arbeiten?
Das Problem das ich jetzt sehe ist, dass Props und State asynchron werden können.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Danke für den Hinweis – ja, du hast recht.
So kann es zu inkonsistentem Verhalten kommen.
Ich werde das umstellen und arbeite weiterhin mit defaultValue wie zuvor, damit Props und State synchron bleiben.


@Watch('_color')
public validateColor(value?: Stringified<PropColor>): void {
// Fallback auf SCSS-Styles, wenn _color nicht gesetzt ist
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die Kommentare müssten bitte alles noch auf Englisch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich werde sie auf Englisch umstellen.


this.styleVars = {
'--kol-badge-bg-color': colorPair.backgroundColor,
'--kol-badge-text-color': colorPair.foregroundColor as string,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ist nicht in diesem PR neu entstanden, aber ich vermute die type assertion ist nicht mehr notwendig?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Danke für den Hinweis! Da foregroundColor aktuell als Stringified getypt ist, ist die Type Assertion (as string) momentan noch notwendig, um TypeScript zufriedenzustellen.
Gerne schaue ich mir das genauer an, wenn wir ColorPair vereinfachen und handleColorChange() so anpassen, dass dort immer ein eindeutiger string zurückgegeben wird.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Danke für die Rückmeldung! Das muss nicht in diesem PR angefasst werden. Es war mir nur ins Auge gesprungen, weil backgroundColor und foregroundColor verschieden behandelt werden.

@deleonio
Copy link
Copy Markdown
Contributor

deleonio commented Jul 15, 2025

Leider abgelehnt - Danke @aTTiCuZ-CodingPage.

@deleonio deleonio closed this Jul 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Badge: style-Attribute in Komponente durch Basis-Styles ersetzen

3 participants