Skip to content

chore: add hover background to table rows in admin pages #35068

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

Closed
wants to merge 1 commit into from

Conversation

BLumia
Copy link
Member

@BLumia BLumia commented Jul 14, 2025

This is a small QoL change to make the admin easier to do their management job. This makes knowing which row is the correct row to operate easier by simply checking the hover background, especially when they have a wide screen.

Sample:

image

The 2nd row is hovered, so before the admin clicks the "delete" button, they can know Magical8bitPlug2 is the one that the "delete" operation will be applied to.

This is a small QoL change to make the admin easier to do their management job. This makes knowing which row is the correct row to operate easier by simply checking the hover background.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 14, 2025
@wxiaoguang
Copy link
Contributor

Since they are still "ui table" elements, then they need this: https://fomantic-ui.com/collections/table.html#selectable-row

And we can't use .admin .table>tbody>tr:hover for "all" tables, it is too broad and not every table needs that "hover highlight".

@wxiaoguang
Copy link
Contributor

And we can't use .admin .table>tbody>tr:hover for "all" tables, it is too broad and not every table needs that "hover highlight".

For example, at least, you broke this table:

image

@BLumia
Copy link
Member Author

BLumia commented Jul 14, 2025

Since they are still "ui table" elements, then they need this:

The selectable class somehow doesn't work as intended while my testing. Will look into it later (we might need to update table.css to add support for selectable?).

And we can't use .admin .table>tbody>tr:hover for "all" tables, it is too broad

I agree. I added .admin to make the scope limited to tables on admin pages, but seems still too board because I previously didn't notice:

For example, at least, you broke this table:
image

Yeah, sorry I didn't notice that before.

I'll convert this PR as WIP to address the issue later.

@BLumia BLumia marked this pull request as draft July 14, 2025 08:38
@BLumia
Copy link
Member Author

BLumia commented Jul 14, 2025

Let's move to #35072 since it involves change related to table.css, and other non-admin pages can get benefit from the change (to make use of the selectable css class) too.

@BLumia BLumia closed this Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants