Page MenuHomePhabricator

Table: Fix sort button padding when column headings span two lines
Closed, DeclinedPublic

Description

For sortable columns, a button element is meant to take up the entire width and height of the <th> of that column to improve UX of sorting a column. Interactive styles are placed directly on the button element, so it is important that the button element and the <th> are the same size.

When a column name spans two lines (any column name, not just the sortable one), it disrupts these styles and the button no longer takes up the full height of the <th>:

image.png (396×1 px, 49 KB)


Acceptance criteria

  • Sort button styles are fixed so that the button takes up the full height of the <th> regardless of column heading wrapping

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
AnneT changed the task status from Open to In Progress.May 15 2024, 1:03 PM
AnneT claimed this task.

I haven't been able to find a cross-browser solution to this:

  • The easiest solution for getting an element to stretch to the height of its parent is using flexbox, but we can't set display: flex on any of the table elements
  • We can't absolutely position the button - this only works if there's a non-sortable column that is as tall or taller than the sortable column(s). For the example on the demo page where all the columns are sortable, the buttons collapse (since their padding is ignored) and the thead layout breaks
  • To set height: 100% on the button, its parent element must have a height. I haven't figured out a cross-browser way to set a height on some of the table elements. For example, I tried setting an arbitrary height on the tr (like 1px) which gets ignored, then height: 100% on both the th and sort button. This works in FF, but in Chrome you need height: inherit on the th instead.

I also don't want to do something hacky that could break with further variations in the thead. Instead, I think it might be best to decline this task and accept that the button will not be full-height if a column heading wraps to multiple lines. The button is still vertically centered and has a decent amount of vertical padding, so it doesn't look that bad.

@Sarai-WMDE what do you think?

Hey @AnneT. Thanks for the catch and for trying out the different approaches 🙏🏻

I gave it a go and the only way I could make the button stretch consistently across browsers is by applying height:100% to both said button and <table>, to ensure inheritance. It sounds extremely hacky, and I'm unsure of which consequences this can have (I could only try this using DevTools with the table With selection and sort example, so my tests are very limited). Do you think this is worth trying?

button-height.gif (500×1 px, 727 KB)

Change #1034548 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] Table: Force sort buttons to be full-height

https://gerrit.wikimedia.org/r/1034548

@Sarai-WMDE that works at least in FF, Chrome, and Safari! I've opened a patch so we can do further testing and ask Volker if there's any risks here. Thanks for finding this!

Change #1034548 abandoned by Anne Tomasevich:

[design/codex@main] Table: Force sort buttons to be full-height

Reason:

Not worth the risk

https://gerrit.wikimedia.org/r/1034548

@Volker_E brought up some valid concerns in the patch. I have not thought of a way to handle this that is worth the risk, especially since the issue itself is not that bad, so I suggest we decline this task

Thanks, Anne! I saw the patch and Volker's comments. I still haven't been able to reproduce any of the potential stretching, overflow or unwanted scroll issues that could be caused by applying a 100% height to the <table> element.

This might be due either to the fact that said element is safe due to being wrapped by two other divs (which height will always be determined by the available content), or due to the fact that I'm only able to test this in a controlled layout environment (demo page and sandbox). I agree with the concerns and wouldn't want to compromise the robustness and adaptability of the table to different contexts, so I'm fine with discarding this approach and trying to find a different solution.

Thanks @Sarai-WMDE! The DST discussed this and chose to decline this task as we don't believe the effort is worth the benefit at this point. If we determine in the future that completing this task is important, we can reopen it.