-
Notifications
You must be signed in to change notification settings - Fork 442
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(SelectableParticipant): restyle component #13017
Conversation
f3433f8
to
22601ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks better and more accessible. but some changes are required:
- A problem with ID
- A problem with input position
<span class="selectable-participant__content"> | ||
<span class="selectable-participant__content-name"> | ||
{{ participant.displayName }} | ||
</span> | ||
<span v-if="participant.statusMessage" | ||
class="selectable-participant__content-subname"> | ||
{{ participant.statusIcon + ' ' + participant.statusMessage }} | ||
</span> | ||
</span> | ||
|
||
<IconCheck v-if="selected" class="selectable-participant__check-icon" :size="20" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's sad, we cannot use Cannot we use NcListItem
here? Because of the check mark? Even with extra actions slot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the problem was in the <a>
tag? Otherwise we can try to hide input in extra slot, I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Postponing until NcListItem is a11y-aproppriate to do that. Then we could migrate this component as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yet, we are using NcListItem
in selectable participants in new conversation dialog. Can we use the same ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to do it with separate pr, to split the changes
22601ff
to
abe6b6c
Compare
abe6b6c
to
ce96b16
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
ce96b16
to
570bc32
Compare
/backport to stable30 |
☑️ Resolves
🖌️ UI Checklist
🖼️ Screenshots / Screencasts
🚧 Tasks
🏁 Checklist