Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of SingleSelect with filter function #6546

Open
wants to merge 30 commits into
base: develop
Choose a base branch
from

Conversation

anicyne
Copy link
Contributor

@anicyne anicyne commented Jun 14, 2024

Refs: #3694

@anicyne anicyne linked an issue Jun 14, 2024 that may be closed by this pull request
@anicyne anicyne marked this pull request as draft June 14, 2024 13:46
Copy link
Contributor

github-actions bot commented Jun 14, 2024

@anicyne anicyne marked this pull request as ready for review June 19, 2024 11:35
@anicyne anicyne marked this pull request as draft June 19, 2024 11:37
@anicyne anicyne marked this pull request as ready for review June 19, 2024 14:13
@sdvg sdvg self-requested a review June 21, 2024 11:33
@sdvg
Copy link
Member

sdvg commented Jun 21, 2024

Ich habe zunächst mal getestet und mir sind noch folgende Punkte aufgefallen:

  1. Wenn ich mit den Pfeiltasten durch die letzten 2-3 Optionen navigiere, scrollt die gesamte Seite mit.
  2. Wenn der Pfeil-Button fokussiert ist, kann ich ihn nur mit der Leertaste, aber nicht mit Enter betätigen. Ist das gewollt?
  3. Ich kann die fokussierte Option nicht mit der Maus verschieben. Es kann praktisch zwei fokussierte Elemente gleichzeitig geben (eins mit Tastatur und eins mit Maus ausgewählt). Ein natives <select> verhält sich hier anders.
image
  1. Man kann die Komponente auch fokussieren, wenn sie eigentlich deaktiviert ist. Entweder durch Tabben oder wenn man auf den Pfeil klickt:
image
  1. Sollten wir eine Meldung anzeigen, wenn es keine Suchergebnisse gibt? Aktuell wird die Liste einfach ausgeblendet und man könnte denken, es handelt sich um eine Combobox, die freie Eingaben erlaubt (Wert wird erst gelöscht, wenn ich auf eine andere Stelle der Seite klicke? Das sollte vermutlich auch auf blur funktionieren).
image
  1. Bei einem Text-Eingabefeld kann man normalerweise auf den Text doppelklicken, wodurch dieser markiert wird. Versucht man das bei dieser Komponente, öffnen und schließen sich die Optionen und der Fokus geht verloren.

  2. Bei einem Klick auf das X geht der Fokus verloren.

  3. Im Default-Theme sind die Radio-Buttons als solche erkennbar. Wurde das irgendwo definiert oder besprochen? Kommt mir erst mal falsch vor.

image

@sdvg
Copy link
Member

sdvg commented Jun 27, 2024

Neuer Test:

  1. Ich bekomme nicht immer Vorschläge angezeigt, wenn ich etwas eintippe:
image
  1. Wenn ich bereits etwas eingetippt habe und dann den Pfeil klicke, wird meine Auswahl gelöscht.

  2. Die Radio-Buttons sind jetzt ausgeblendet, nehmen aber noch Platz ein:

image
  1. Wenn der Pfeil-Button fokussiert ist, kann ich ihn nur mit der Leertaste, aber nicht mit Enter betätigen. Ist das gewollt?

Das ist weiterhin so. Auf <Enter> wird jetzt das Eingabefeld fokussiert.

  1. Bei der deaktivierten Komponente kann ich weiterhin den X-Button klicken und den Tooltip für den Pfeil aktivieren:
image
  1. "Sollten wir eine Meldung anzeigen, wenn es keine Suchergebnisse gibt? Aktuell wird die Liste einfach ausgeblendet und man könnte denken, es handelt sich um eine Combobox, die freie Eingaben erlaubt (Wert wird erst gelöscht, wenn ich auf eine andere Stelle der Seite klicke? Das sollte vermutlich auch auf blur funktionieren)."

Dieser Punkt ist noch offen.


Die übrigen Punkte sind erledigt, danke!

@anicyne anicyne force-pushed the 3694-feature-singleselect-component branch from ed62da1 to fc7bc9f Compare July 2, 2024 09:42
@anicyne anicyne force-pushed the 3694-feature-singleselect-component branch from 07a0e84 to 6316653 Compare July 2, 2024 14:04
@anicyne anicyne force-pushed the 3694-feature-singleselect-component branch from 6316653 to 25063a5 Compare July 2, 2024 14:09
packages/components/src/schema/components/single-select.ts Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Steht vermutlich nicht im Ticket, aber wir sollten neue Inputs immer auch in den entsprechenden Szenarien ergänzen:

  • inputs-get-value
  • static-form
  • focus-elements

Copy link
Member

Choose a reason for hiding this comment

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

Könntest du die Events anhand der Szenarien bitte einmal durchtesten und korrigieren:

  • bei onInput bekomme ich numbers statt der Werte.
  • ich sehe teilweise Exceptions.
  • bei meinem letzten Test bekam ich false als Wert.

Falls du die Fehler nicht reproduzieren kannst geb gerne noch mal Bescheid, dann dokumentiere ich die Schritte genauer!

image

Copy link
Member

@sdvg sdvg Jul 4, 2024

Choose a reason for hiding this comment

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

static-form konnte ich nicht testen, weil das SingleSelect hier keine Auswahlmöglichkeiten hat.
image

@anicyne anicyne force-pushed the 3694-feature-singleselect-component branch from 5f47669 to e5ddecb Compare July 4, 2024 09:05
@anicyne anicyne requested a review from sdvg July 4, 2024 10:26
private _showNoResultMessage: boolean = false;

@Listen('click', { target: 'window' })
handleWindowClick(event: MouseEvent) {
Copy link
Member

Choose a reason for hiding this comment

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

Das scheint mir noch nicht zuverlässig zu funktionieren. Wenn ich z.B. im inputs-get-value-Scenario auf den getValue button drücke, bleibt die "keine Ergebnisse"-Meldung offen:
image

Copy link
Member

@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.

Mir ist leider auch noch eine Regression aufgefallen:
Ich kann den Pfeil für die Auswahlliste nicht mehr mit Enter öffnen. Hierbei wird jetzt das Eingabefeld fokussiert.

@anicyne anicyne requested a review from sdvg July 15, 2024 07:57
@sdvg
Copy link
Member

sdvg commented Jul 15, 2024

Es gibt noch ein Problem mit dem Fokus-Verschieben mit der Maus. Zum Reproduzieren:

  1. Optionen öffnen
  2. Mit der Maus über die erste Option fahren (nicht klicken)
  3. Pfeiltaste nach oben drücken

Soll: Die letzte Option in der Liste ist fokussiert.
Ist: Die Option, die jetzt unter dem Cursor liegt, ist fokussiert.

screenshot 2024-07-15-15 31 29

@sdvg
Copy link
Member

sdvg commented Jul 15, 2024

Mit getValue stimmt noch etwas nicht: Wenn ich den Wert mit einem Klick auf "X" lösche, bekomme ich immer noch den vorherigen Wert:
screenshot 2024-07-15-15 36 54@2x

✅ Erledigt


private readonly catchRef = (ref?: HTMLInputElement) => {
this.refInput = ref;
propagateFocus(this.host, this.refInput);
Copy link
Member

Choose a reason for hiding this comment

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

Hier müssen wir noch auf die neue Methode, kolFocus umstellen. Das kann ich übernehmen.

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.

New Component: SingleSelect mit Filterfunktion
2 participants