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

Improvements for the ClayPanel component #5624

Open
veroglez opened this issue Jul 6, 2023 · 6 comments
Open

Improvements for the ClayPanel component #5624

veroglez opened this issue Jul 6, 2023 · 6 comments
Labels
comp: clay-css Issues related to Clay CSS type: question

Comments

@veroglez
Copy link
Member

veroglez commented Jul 6, 2023

Hi guys!

From the Echo team we are replacing all our Collapses with ClayPanel component, and doing this migration several questions have arisen:

When an item is clicked a focus appears, could we implement c-inner so that the focus only appears when the element is focused?

Screen Shot 2023-07-06 at 2 32 24 PM

Also, we have also noticed that the styles are not the same as those defined in Lexicon for this component (see the following images). Is it possible to implement them?

Current style
Screen Shot 2023-07-06 at 2 33 28 PM

Styles defined by Lexicon
image

Thank you very much in advance!

@matuzalemsteles
Copy link
Member

When an item is clicked a focus appears, could we implement c-inner so that the focus only appears when the element is focused?

Yeah, I think it's possible! not sure if this conflicts with the new accessibility menu rule that this will be activated by the user with focus visible. Thoughts @ethib137 @marcoscv-work

Also, we have also noticed that the styles are not the same as those defined in Lexicon for this component (see the following images). Is it possible to implement them?

cc @pat270

@marcoscv-work
Copy link
Member

marcoscv-work commented Jul 7, 2023

@matuzalemsteles yeah, no problem on removing focus for mouse interaction. that's something Lexicon always wanted and we can apply as much as possible.
WCAG 2.2 will be more specific about this, but it also specify it applies for keyboard interaction / screen reader (they have their own focus):
https://www.w3.org/WAI/WCAG22/Understanding/focus-appearance-minimum.html

@ethib137
Copy link
Member

ethib137 commented Jul 7, 2023

I think the questions we need to be asking are the following:

  • What should the default behavior be?
    • ie: focus-visible-only (only display focus for keyboard interaction) or focus-visible (display focus for both keyboard and mouse interactions)
  • What should the accessibility menu setting be (Other settings for Reference)?
    • Label?
    • Description?
    • Default Value?
    • CSS Class?

From the discussion thus far it sounds like we are proposing the following:

  • What should the default behavior be?
    • focus-visible-only (only display focus for keyboard interaction)
  • What should the accessibility menu setting be?
    • Label: Focus Indicator
    • Description: Show the focus indicator when either the mouse or keyboard is used.
    • Default Value: False
    • CSS Class: "c-prefers-focus-indicator"

Related Issue

@pat270 pat270 added the comp: clay-css Issues related to Clay CSS label Jul 7, 2023
@marcoscv-work
Copy link
Member

I think for this question @veroglez is speaking about, we need to use focus-visible-only (only for keyboard). we already had this DXP component working in this way in portal and they are trying to use the clay component so for this case I think is clear.

From a design perspective we can use always focus-visible-only. since our interactive components should have always hover state.

Speaking about accessibility menu:

We have components that are showing the focus sometimes with mouse interactions. Lexicon dint wanted this but it was a initial technical limitation, it's something understandable, we all know that adding c-inner for all clay components/DXP components/taglibs/custom markup, etc.. would be a complex task, for that reason we were looking for a way to completely deactivate this effect.

If now we can do it focus-visible-only (only for keyboard) globally without the need of use c-inner, I would go for that as a default value in DXP and I would redefine the accessibility menu option, we could change it of just remove it.

@matuzalemsteles
Copy link
Member

If now we can do it focus-visible-only (only for keyboard) globally without the need of use c-inner, I would go for that as a default value in DXP and I would redefine the accessibility menu option, we could change it of just remove it.

I think this comes back into the discussion about this in the implementation of PR #5006.

@matuzalemsteles
Copy link
Member

LPS Ticket https://liferay.atlassian.net/browse/LPS-192228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: clay-css Issues related to Clay CSS type: question
Projects
None yet
Development

No branches or pull requests

5 participants