Table component sorts and rerender on onmouseover and onmouseout

[AndreeWille]

Describe the bug

On a sortable column when you put the mouse over or away from the header of the column that is sortable then the table gets sorted and rerendered. Which means that when a user clicks on a header with a the property sortable = true and afterwards moves the mouse away from the header then the table gets sorted 3 times. First time on onMouseOver, 2nd time on onClick and 3rd time on onMouseOut.

Steps to reproduce

Steps to reproduce the behavior:

1. Add the prop sortable = true to a table header (dont fort get to add a sortLabel as well)
2. Either add a custom sort function with a console.log or start the profiler from the react dev tools.
3. Click on the sortable header and move the mouse out of the header.
4. Either you see 3 console.log statements in the console or 3 rerender of the table component in react dev tools.

Expected behavior

Table will only be sorted once when a user clicks on the table header.

Specifications

• Version: 4.5.1
• Browser: Latest Firefox and Latest Chrome
• OS: Mac OS 12.0.1

Additional context

[robinmetral]

Thanks for reporting @AndreeWille, I can reproduce easily using by logging onSortBy calls in our custom sort story.

The issue

This is happening because we listen to mouseenter and mouseleave events on the table header (source) in order to highlight the entire column to be sorted when the header cell is hovered.

The mouse enter and leave events update the sortHover in the Table's local state, which triggers a component update and calls the sort function via getSortedRows (source).

The solution(s)

One way we can solve this is by avoiding to call getSortedRows inside the render method and rather move the sorted rows into state. PR on the way!

Alternatively, we could also remove the logic to highlight the entire column when the header cell is hovered: I don't think that this behavior is a big usability improvement, and it definitely worsens the component's performance and maintainability (as illustrated by this very issue), not to mention that it breaks the comparable experience principle of inclusive design by not implementing the same behavior for keyboard users. If we were to remove these mouseenter/mouseleave handlers, only the header cell would be highlighted when it is hovered.

---

Side note: this is a legacy component and it will be entirely rewritten in an upcoming release. We will consider future bug fixes to the extent possible, but we can't prioritize them at this time. PRs are always welcome though 🙌

[robinmetral]

☝️ a PR with a suggested fix is here. Try it on the deploy preview.

[robinmetral]

Closed by #1292, the fix is in @sumup/circuit-ui@4.5.3.

Thanks again for the report @AndreeWille!

[AndreeWille]

Thank you @robinmetral for fixing this so quickly!

[robinmetral]

The fix for the Table re-render had a bug, which was addressed in #1306.

However it looks like it reintroduced the re-render on Table header hover: reopening.

[AndreeWille]

I had a look into this and came to the conclusion that the issue is that hovering over a table header changes the state of the table in order to store the column index, which results in rerendering the table.
From my understanding the state is only for higlighting the column, is this correct?

If yes, then i would be happy to prepare a pr where i remove the state change but instead add the html col tag according to the amount of headers and then dynamically set the background color on col when hovering. This should have the same effect.

Would this make sense to you?

[robinmetral]

I had a look into this and came to the conclusion that the issue is that hovering over a table header changes the state of the table in order to store the column index, which results in rerendering the table.

Yes, exactly

From my understanding the state is only for higlighting the column, is this correct?

I'm not sure, but we can try to remove it! A PR would be awesome, then we can make sure all tests pass and test the component in the Storybook. Let me know if you need help getting set up with the repo 🙂 (Start here)

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Add a comment explaining why the issue is still relevant to prevent it from being closed.