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(NcAvatar): make it a span phrasing element #4674

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Oct 19, 2023

It is required to allow using avatar as a phrasing content, for example, as a button content.

☑️ Resolves

Error: Element div not allowed as child of element button in this context. (Suppressing further errors from this subtree.)

Sometimes we use NcAvatar as a button content, for example, as a Popover trigger on the main UserMenu.

HTML does not allow this. Button should have only so-called phrasing (aca inline text) content.

This PR makes NcAvatar a span instead of a div.

For some places it doesn't change anything in the layout, because it has display: inline-block, or position: absolute. In other places display: block; was implicitly added.

Alternative solution: make NcHeaderItem not a <button> but a div[role=button].

#4675

🖼️ Screenshots

No visual changes

User Menu Comments Sharing Talk
image image image image

🚧 Tasks

  • Replace all div with a span
  • Update styles, add display: block; where it is required.

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews feature: avatar Related to the avatar component accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Oct 19, 2023
@ShGKme ShGKme added this to the 8.0.0 milestone Oct 19, 2023
@ShGKme ShGKme self-assigned this Oct 19, 2023
It is required to allow using avatar as a phrasing content, for example, as a button content.

Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme ShGKme force-pushed the fix/avatar-as-phrasing-element branch from 184c60d to 4e05761 Compare October 19, 2023 14:14
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

makes sense

@Pytal Pytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 19, 2023
@Pytal Pytal merged commit 7d141d1 into master Oct 19, 2023
@Pytal Pytal deleted the fix/avatar-as-phrasing-element branch October 19, 2023 17:45
@Pytal Pytal mentioned this pull request Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: avatar Related to the avatar component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants