rusackas commented on issue #43084: URL: https://github.com/apache/superset/issues/43084#issuecomment-6168702612
Hey @ali-rajan, thanks for writing this up, that's a solid plan and matches how `tableOptions` already flows through `transformProps.ts` -> `PivotTableChart.tsx` -> `TableRenderers.tsx` for settings like this. I'd say go for it! One thing worth knowing is that the color formatting bug dosu described above (issue 1) is already fixed on `master` via #43481, so you're only on the hook for the row label truncation half (issue 2). Might be worth filing a separate issue for that at some point, but I'm not worried about that. A couple of small notes on the implementation: Rather than putting `max-width`/`overflow: hidden`/`text-overflow: ellipsis` directly on the `<th>`, I'd wrap the label text in an inner `span` or `div` that carries those styles. Table cells under the default `table-layout: auto` don't reliably respect `max-width` for truncation across browsers, especially with the `rowSpan`/`colSpan` these header cells already use. It'd be worth testing by hand with a multi-level row grouping (2+ columns in Rows) and the subtotal toggle arrow, so the arrow icon doesn't get clipped along with the text. Also, `plugin-chart-table` already solved basically this same problem (`.dt-truncate-cell` in its `Styles.tsx`), though it expands on hover instead of using a tooltip. Not saying you need to copy that inplementation, a `title` attribute is simpler and fine, just flagging it so the UX doesn't end up feeling wildly different between the two table-ish chart types. Always trying to keep Superset consistent/themed/DRY when possible. Everything else, renderer tests for unset vs. configured width and the tooltip content, sounds right, and `test/react-pivottable/tableRenders.test.tsx` is the right home for those. Happy to take a look once you've got a draft PR up. Thanks in advance! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
