Normalize _value to array in kol-select to handle single values

[donchi-donald]

No description provided.

[deleonio]

Die Änderung würde ich gerne noch überdenken.

[sdvg]

Ich habe mir erlaubt, die Commits in einen neuen Branch innerhalb des Repos zu verschieben, damit alle GH Actions ausgeführt werden und damit ich ggf. daran weiter arbeiten kann: #7795

[deleonio]

@sdvg, wie gehen wir mit diesem PR um?

[sdvg]

@sdvg, wie gehen wir mit diesem PR um?

Den würde ich schließen, sobald #7795 gemerged ist. Für den Moment würde ich ihn offen lassen, falls noch Änderungen durch @donchi-donald notwendig sind.

@deleonio Hattest du zu #7795 denn noch Anmerkungen oder wäre das eigentlich bereit, gemerged zu werden? Der angesprochene Punkt mit der Dokumentation bzw. den Beispielen wäre in einem neuen PR für die Docs zu erledigen.

[donchi-donald]

Ich habe die _value-Angabe im Beispiel entfernt.

Jetzt stellt sich für mich die Frage:

Soll ich die Dokumentation im Rahmen dieses Tickets (#7758) anpassen, oder wird dafür ein separates Ticket erstellt?

Hintergrund: Der Select-Komponent unterstützt jetzt sowohl einfache string-Werteals auch Arrays von string (string[]).

Ich warte gerne auf Feedback, wie wir die Doku an dieser Stelle aktualisieren sollen. 😊

[sdvg]

Ich habe die _value-Angabe im Beispiel entfernt.

Das war aus meiner Sicht eigentlich nicht notwendig, ich hatte die Snapshots (im gespiegelten PR) schon dahingehend aktualisiert. Aber warten wir mal ab, was Martin sagt :)

Soll ich die Dokumentation im Rahmen dieses Tickets (#7758) anpassen, oder wird dafür ein separates Ticket erstellt?

Neues Ticket schadet hier auf jeden Fall nicht, kannst du gerne anlegen.

Hintergrund: Der Select-Komponent unterstützt jetzt sowohl einfache string-Werteals auch Arrays von string (string[]).

Mir ging es in den Docs vor allem darum, die Beispiele zu vereinfachen (_value={['Mr.']} wird zur einfacheren Version _value="Mr."). Ein Absatz, der noch mal kurz beschreibt, dass beides möglich ist, schadet aber sicher auch nicht!

[donchi-donald]

Danke für dein Feedback!
Ich habe jetzt ein separates Ticket für die Doku angelegt, wie vorgeschlagen: public-ui/public-ui.github.io#328

Kannst du bitte kurz schauen, ob das so passt?

[sdvg]

Ich habe direkt im Doku-Ticket noch einen Kommentar geschrieben.

[sdvg]

_multiple_ bringt hier einen interessanten Aspekt hinein, Gedanken dazu:

1. Hintergrund zur _multiple-Prop

• Die _multiple-Prop wird aktuell einzig dazu verwendet, das native <select>-Element mit multiple auszustatten.
• Es gab bislang keine enge Verknüpfung zwischen _multiple und dem Datentyp von _value (Single-Value vs. Array-Value).

2. Typ-Ermittlung von _value

• Statt sich auf den Typ von _multiple zu verlassen, sollte man besser den initialen Wert prüfen (siehe kol-input-number, kol-input-date).

3. Rückwärtskompatibilität

• Für bestehende Implementierungen sollten beide Varianten weiterhin funktionieren:
  1. Single-Value:
     <KolSelect _value="single value" />
  2. Array-Value:
     <KolSelect _value={["array value"]} />
• Die Prop _multiple darf dabei nicht implizit den Rückgabewert verändern, sondern sollte nur das Attribut an das native <select> weiterreichen.

4. Problemfall: Kombination von _value="single value" und _multiple

<KolSelect _value="single value" _multiple />

• Hier ist unklar, ob der "single value" oder ein Array zurückgegeben werden soll.
• In diesem Fall macht es m.E. keinen Sinn, einen einfachen String als Rückgabewert zu haben, wenn _multiple gesetzt ist.

5. Mögliche Lösungsansätze

1. Fehler/Warnung ausgeben

   • Wenn _multiple={true} und _value ist kein Array, würde eine Runtime-Exception oder zumindest eine Warnung im Log erscheinen.

2. Nur noch konsistente Kombis unterstützen (Breaking Change)

   • _multiple={false} → nur Single-Value
   • _multiple={true} → nur Array-Value
   • Vorteil: Typensicherheit und klare Erwartung, Nachteil: möglicher Bruch mit existierendem Code.

Fazit/Empfehlung

• Meiner Meinung nach sollten wir kurzfristig zumindest eine Warnung ausgeben, wenn jemand _multiple mit einem Single-Value nutzt.
• Langfristig könnte man überlegen, auf eine strengere Typisierung umzusteigen (Option 2), sobald ein Major-Release ansteht.

@deleonio @laske185 Wollt ihr hierzu noch Input geben?

[donchi-donald]

Vielen Dank für das ausführliche Feedback – das hilft mir, den Zusammenhang zwischen _multiple und _value jetzt deutlich besser zu verstehen. Ich sehe ein, warum eine klare Trennung bzw. konsistente Handhabung hier wichtig ist.

Ich werde die entsprechenden Anpassungen sofort vornehmen und das Verhalten entsprechend korrigieren, sodass bei Nutzung von _multiple ein Array erwartet wird bzw. zumindest eine Warnung erscheint, wenn dies nicht der Fall ist.

Danke nochmal für die Erklärung!

[sdvg]

Hi @donchi-donald

Wir haben das Thema heute morgen noch einmal im Team besprochen:

Der Konsens ist jetzt, dass doch die _multiple-Property berücksichtigt, und sich auf den (erwarteten) _value auswirken soll. Wir wollen also praktisch den oben beschriebenen Lösungsansatz 2 umsetzen.

Das heißt auch, dass wir die Validierung entsprechend anpassen müssen:
-> Ändern sich _value oder _multiple, muss der _value darauf validiert werden, dass er zur _multiple-Property passt.

• _value={["multi"]} _multiple ✅ valid
• _value={"single"} _multiple ❌ invalid
• _value={"single"} _multiple={false} ✅ valid
• _value={"single"} _multiple ❌ invalid

Danke für deine Geduld mit dem hin-und-her, das Ticket hätte initial besser durchdacht sein können.

[donchi-donald]

Hi @sdvg ,

vielen Dank für das Update und die Klarstellung.

Tatsächlich hatte ich die Lösung bereits unter Berücksichtigung des initialen Wertes implementiert. Ich verstehe nun aber, dass zusätzlich auch die _multiple-Property aktiv berücksichtigt und bei Änderungen entsprechend validiert werden soll.

Ich werde meinen Code zeitnah aktualisieren, um diese Validierungslogik mit einzubeziehen: Wenn sich _value oder _multiple ändern, wird _value so validiert, dass es konsistent zur _multiple-Property passt.

Vielen Dank nochmals für das konstruktive Feedback und die klare Richtung!
Ich passe die Lösung entsprechend an.

Beste Grüße