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

🐞 Bug: aria-current bei Navigations-Menüs ohne Unterpunkte wird nicht gesetzt #2043

Closed
tim-laue opened this issue Jan 16, 2023 · 8 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@tim-laue
Copy link
Contributor

tim-laue commented Jan 16, 2023

Fehlermeldung

Beschreibung des Fehlers

Siehe voriges Issue zum aria-current Attribut. In unserer Appplikation, die nur eine flache Liste von Menüpunkten hat, wird das Attribut nach wie vor nicht gesetzt.

Reproduktion

Ist in der Code Sandbox direkt sichtbar.

Erwartetes Verhalten

Dass das Attribut auf "page" gesetzt wird.

Screenshots

bugnav1

bugnav2

Desktop:

  • Betriebssystem: Windows
  • Browser: Firefox und Chrome
  • Kolibri-Version 1.1.15

Zusätzlicher Kontext

Hansell hat hierzu den Quellcode schon einmal untersucht. Hier wird, wenn der Menüpunkt keine Children hat, das aria-current Attribut explizit nicht gesetzt. Warum ist das so?

@tim-laue tim-laue added the bug Something isn't working label Jan 16, 2023
@deleonio deleonio added the good first issue Good for newcomers label Jan 16, 2023
@deleonio
Copy link
Contributor

deleonio commented Jan 16, 2023

@tim-laue - Ihr findet bestimmt schnell die fehlende Stelle. Könnt Ihr bitte forken und einfach einen PR stellen?

https://github.com/public-ui/kolibri/blob/main/packages/library/src/components/nav/component.tsx

@tim-laue
Copy link
Contributor Author

tim-laue commented Jan 16, 2023

@deleonio - An der betreffenden Stelle im Code steht ein Kommentar der explizit sagt, dass aria-current hier nicht gesetzt werden soll. Das können wir uns leider nicht erklären, sodass ich Stand jetzt ohne Weiteres keinen PR dazu erstellen kann.

@deleonio
Copy link
Contributor

Hi @tim-laue, schau mal ca. 20 Zeilen weiter oben - dort wir aria-current gesetzt. Es ist quasi genau verkehrt herum.

@tim-laue
Copy link
Contributor Author

@deleonio PR ist erstellt.

@deleonio deleonio added this to the v1.3 milestone Jan 25, 2023
@deleonio
Copy link
Contributor

removed with v1.3 - ist schon seit längerem nicht mehr im code verknüpft

@deleonio
Copy link
Contributor

fix with v1.3

@deleonio deleonio modified the milestones: v1.4, v1.3 Jan 25, 2023
@tim-laue
Copy link
Contributor Author

tim-laue commented Feb 2, 2023

@deleonio Habt ihr dieses Feature mit der aktuellen Version schon einmal getestet? Bei uns erscheint nach wie vor kein aria-current.

@deleonio deleonio reopened this Feb 2, 2023
@github-project-automation github-project-automation bot moved this from ✅ Done to 🏗 In progress in KoliBri Board Feb 2, 2023
@deleonio deleonio closed this as completed Feb 2, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in KoliBri Board Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
Archived in project
Development

No branches or pull requests

2 participants