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

fix(journey-maps): display focused controls #1839

Merged
merged 3 commits into from
Mar 17, 2023

Conversation

swiss-chris
Copy link
Contributor

Refs: ROKAS-1364

@swiss-chris swiss-chris marked this pull request as draft March 14, 2023 11:04
@@ -52,7 +52,8 @@
}
}

&:not([data-disabled='true']):active {
&:not([data-disabled='true']):active,
&:not([data-disabled='true']):has(:focus-visible) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

:has() wird leider von Firefox noch nicht unterstützt, aber laut w3c/csswg-drafts#3080 (comment) wird es auch Firefox sehr bald unterstützen.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ist aber schon ein bisschen heikel, da die keyboard navigation auf firefox zurzeit nicht accessible wäre. Hast du eine Info, ab welcher Firefox Browser Version, :has() aktiviert sein wird?

Zudem sollten sich eigentlich gemäss design Richtlinien active und focus state unterscheiden. Ich würde empfehlen, eine focus outline zu verwenden, wie wir das im Lyne design system machen. Siehe https://lyne-storybook.app.sbb.ch und teste z.b. mit Buttons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ist aber schon ein bisschen heikel, da die keyboard navigation auf firefox zurzeit nicht accessible wäre. Hast du eine Info, ab welcher Firefox Browser Version, :has() aktiviert sein wird?

Zudem sollten sich eigentlich gemäss design Richtlinien active und focus state unterscheiden. Ich würde empfehlen, eine focus outline zu verwenden, wie wir das im Lyne design system machen. Siehe https://lyne-storybook.app.sbb.ch und teste z.b. mit Buttons.

Ich habe leider nicht mehr gefunden als das hier https://hacks.mozilla.org/2023/02/announcing-interop-2023/ vom 1. Februar "As well as older features, Interop 2023 also contains some new additions to CSS. Based on feedback from web developers we know that two of these in particular are widely anticipated: Container Queries and parent selectors via :has(). Both of these features are currently being implemented in Gecko; Container Queries are already available to try in prerelease versions of Firefox, and is expected to be released in Firefox 110 later this month, whilst :has() is under active development."

Copy link
Contributor Author

@swiss-chris swiss-chris Mar 14, 2023

Choose a reason for hiding this comment

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

@jeripeierSBB Unser PO bittet darum, dass wir diesen change erst mal so releasen können, im Rahmen eines Bugfixes https://flow.sbb.ch/browse/ROKAS-1364 (soll ich meinen PR als "fix" bezeichnen?). Wir würden beide Punkte aber in kommenden Stories angehen.

  1. Wir würden wahrscheinlich die Buttons umbauen, und <button> als Container stylen und darin ein <span> anstatt wie heute den container div zu stylen und den <button> im <div> drin.
  2. Es gibt noch gar keine neuen Designs für unsere Buttons mit dem Outline, weder in https://lyne-storybook.app.sbb.ch/ noch anderswo. Das wird aber noch nachgeliefert und wir bauen das dann ein.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hallo @swiss-chris
Danke für die ausführliche Antwort. Natürlich könnt ihr das erstmal so mergen, ich bin einfach nicht sicher ob die accessbility prüfung stand halten wird (und aus diesem Grund müsst ihr diese Änderung ja überhaupt erst machen ;-) Bei sbb.ch / Lyne haben wir relative strenge Anforderungen und sowohl Chrome, Safari und Firefox müssen auf Desktop unterstützt werden, wobei mit diesem PR eben nur 2/3 unterstützt werden würden. Die andere Richtlinie mit dem unterschiedlichen Fokus und Active style ist glaube ich kein Killerkriterium.

Zu 1) das würde wahrscheinlich funktionieren, zu 2) ich weiss gar nicht wo eure designs herkommen. Ansprechspartner zum Thema Focus wäre sicher @mcilurzo .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Super, danke. Den Manuel hatte unsere UX Designerin heute diesbezüglich schon angeschrieben.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Um auch noch kurz etwas dazu zu sagen; Aussage von Firefox ist erstes Halbjahr: https://bugzilla.mozilla.org/show_bug.cgi?id=418039
Damit wäre wahrscheinlich der Zeitplan von sbb.ch nicht gewährleistet, falls der Release davon eher in Q2 fällt.
Aber wie Jeri bereits gesagt hat, wir können das schon mal mergen.

@github-actions
Copy link

Preview ready from 08292d7 at 14.3.2023, 11:06:22:

@swiss-chris swiss-chris marked this pull request as ready for review March 14, 2023 14:50
@swiss-chris swiss-chris changed the title feat(journey-maps): display focused controls fix(journey-maps): display focused controls Mar 14, 2023
@kyubisation kyubisation merged commit 9ec4e6f into main Mar 17, 2023
@kyubisation kyubisation deleted the feature/cdi/ROKAS-1364-display-focused-controls branch March 17, 2023 14:34
sbb-design-systems-bot bot pushed a commit that referenced this pull request Mar 17, 2023
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.

3 participants