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

WIP: Kompletten Bildschirm für Karte nutzen #38

Merged
merged 10 commits into from Oct 26, 2022

Conversation

CherryKitten
Copy link
Contributor

@CherryKitten CherryKitten commented Oct 25, 2022

Funktionalität ist soweit fertig, bei kleinen Bildschirmen ist der Header erst versteckt und es gibt einen Floating Action Button um den Header anzuzeigen/wieder zu verstecken.

Was noch fehlt:

  • Accessibility
  • Text für den Button/Screenreader-Text. Ich hätte eigentlich "Menü" und "Menü anzeigen/ausblenden" gedacht, aber es gibt ja bereits ein anderes Element das als Menü beschrieben wird. Header wäre glaube ich für Nutzer*innen zu uneindeutig? Ich bin mir unsicher, was da am besten passen würde

Closes #2

@nachtjasmin
Copy link
Member

nachtjasmin commented Oct 25, 2022

Oh wow, danke Sammy! Ich bin echt dankbar, dass du dir da schon so viele Gedanken zu dem Issue gemacht hast. 😊

Bezüglich der Barrierefreiheit: Ich denke, dass "Menü" und "Menü anzeigen/ausblenden" da gut ist. Die zweite Navigation zeigt ja eigentlich nur Informationen an, die könnte dann entsprechend umbenannt werden. Dann könnte das Symbol ein einfaches Info-Symbol sein.

Andererseits könnte es Sinn ergeben, dann alles von der Logik in das Seiten-Menü zu verschieben. @xenein, was meinst du?

Ich weiß nur nicht, wie das mit den aria--Attributen am Besten geht, und ob ein aria-controls="mobile-header" für den Button da ausreicht. Da muss ich mich selbst nochmal einlesen.

@CherryKitten CherryKitten marked this pull request as ready for review October 26, 2022 08:47
Copy link
Member

@nachtjasmin nachtjasmin left a comment

Choose a reason for hiding this comment

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

Danke ^^
Ich hab's dann fix noch ein bisschen refactored, damit der Button sauber ausgerichtet ist. Und noch ein paar Tests ergänzt, damit kann das jetzt gemerged werden. 😊

@xenein
Copy link
Member

xenein commented Oct 26, 2022

falls die Frage nach meiner Meinung noch aktuell ist:
Solang das aus irgendwelchen Gründen nicht riesiger Aufwand ist, kann man das schon machen, wäre sicher nicht verkehrt, seh ich aber nicht zwingend dringend.

@nachtjasmin nachtjasmin merged commit 93751b6 into Queer-Lexikon:main Oct 26, 2022
@nachtjasmin
Copy link
Member

Die Menüs können wir später denk ich immer noch zusammenfügen, aber für's erste dürfte das so funktionieren.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kompletten Bildschirm für Karte nutzen
3 participants