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

Adds: Mouseover text to player status icons #948

Closed
wants to merge 1 commit into from

Conversation

gegelendvay
Copy link
Contributor

To improve the clarity of what each icon represents, I have added mouseover text (using the title attribute) to each icon. This should make it easier for users to understand the purpose of each icon at a glance.

@gegelendvay gegelendvay requested a review from tabarra as a code owner May 18, 2024 19:39
@gegelendvay gegelendvay changed the base branch from master to develop May 18, 2024 22:36
@tabarra
Copy link
Owner

tabarra commented Jun 13, 2024

Thank you for the PR.
These changes do not work as these elements get translated the <svg> tag which does not have the title attribute.
The correct way of implementing this would be to open the igon tag and add a <title> element to them, like so:

<ShieldCheckIcon className={cn('h-5',
    rowData.isAdmin ? 'text-warning-inline' : 'text-muted'
)}>
    <title>Is Admin</title>
</ShieldCheckIcon>

But with that said, and althought I do see the value in this addition, I will still not apply those changes because the player row must be as light as possible due to how much they are spam-rendered, and the fact that those have been up for over two months and so far nobody asked me to explain what are those, so it's probably clear enough for most users.
Thanks again for the PR though!

@tabarra tabarra closed this Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants