fix(DataTable): align items top on text wrapping

[francinelucca]

Closes #12610

Programmatically determine if a Table has a cell that is wrapping text content inside of it, if so apply a new cds--data-table--top-aligned class that will make the table align items top.

My approach here is for any given table cell, measure the textWidth inside a canvas when it is not being wrapped and compare it to the cell's width, if the rendered text is wider than the cell then the text is being wrapped. This was the only way I found to do this, definitely open to suggestion on a better implementation.

Shoutout @guidari for pairing up with me on this 🚀

Changelog

Changed

• Call to new setTableAlignment function in Table component on first render and on resize to determine wether table should be top aligned.
• Add/Modify Table css to align items top and appropriate padding when necessary
• Enforce default lg size in Table

Testing / Reviewing

https://deploy-preview-13525--carbon-components-react.netlify.app/?path=/story/components-datatable-basic--default

1. Go to a DataTable Playground story
2. turn experimentalAutoAlign control on
3. resize window until text is wrapped, confirm items align to the top on text wrapping and that when no text is being wrapped datatable still aligns center and looks as expected.
4. Turn experimentalAutoAlign control off and confirm items do not align to the top when text is wrapping
5. I know it's a lot but please test different variations and sizes 🙏🏻🙏🏻🙏🏻
   Sizes: xs,sm,md,lg,xl
   Variations:

• Expansion
• Selection (Checkbox, RadioButton)
• Actions
• With OverflowMenu

[netlify]

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	d14475f
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-components-react/deploys/6480a3849c44560008fdb44c
😎 Deploy Preview	https://deploy-preview-13525--carbon-components-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[netlify]

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	d14475f
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-elements/deploys/6480a3840c8c440008f8b59d
😎 Deploy Preview	https://deploy-preview-13525--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[alisonjoseph]

I haven't dug into the code yet, but should we hide this functionality behind a prop? it visually changes things, so technically adding this in now could be classified as breaking. Thoughts @francinelucca @tay1orjones

[tay1orjones]

@alisonjoseph We did the feature flag for the new disabled styling for Tile because it was changes to styling of existing selectors.

In this case it's all behind a new classname. Yes in theory there could still be conflicts with downstream consumers' styling but I think the surface area of this is relatively minimal and doesn't warrant a feature flag.

We can always add a feature flag in the future if we receive feedback of it causing unintended consequences downstream.

[tay1orjones]

Kudos for diving into this, it's complicated and difficult to figure out something that works! I'm a bit concerned about the reliability of measuring with canvas and the performance hit of measuring each cell.

If this method proves to be reliable and performant though, I could see it being useful in other situations where we need to intelligently test for truncation when we can't rely on text-overflow: ellipsis. What do you think about moving the measurement logic to a reusable useTextMeasurement hook?

// something like this maybe?
const hasOverflow = useTextMeasurement(myRef);

[tay1orjones]

Iterating over every table cell on every render seems really expensive. Do you think this will be performant enough to use on tables with large datasets that don't use pagination? Might be worth testing to assess the usability impact.

[francinelucca]

yeah I agree it's not the most elegant code out there 😅. I couldn't figure out any other way to do it though, any one cell could be wrapping content, so we need to check all of them - or stop when we find one that is wrapping, which I'm doing but if none of them are wrapping then yeah, it'll run through all-.
I can add a large datatable to test, how many rows/columns are you thinking will be a good enough test?

[tay1orjones]

1000+ rows is where we've seen other performance issues #4338

If we're not 100% confident in the performance, we could consider putting the behavior behind a an experimental prop so users can opt into it rather than it be on by default. Document that it has performance implications, etc

[tay1orjones]

Similar concern here with performance, iterating over every table cell for every resize event could really impact usability if the updates are blocking. I don't think useEvent debounces or throttles the events either.

[tay1orjones]

Thanks for providing those testing stories! The 10000 cells example looks pretty rough

Would it be more applicable for this code to live within TableCell or TableRow and be inside a useLayoutEffect instead of useEffect? I wonder if that would break this work up into multiple tasks instead of one long running one. I also wonder if react might then be able to assist in yielding to the main thread since each task is contained within a multiple components instead of in the Table.

[francinelucca]

@laurenmrice this should be ready for re-review, pending header wrapping confirmation

[alisonjoseph]

This looks great! pending design approval 😄

[francinelucca]

This should be ready for re-rev @laurenmrice

[francinelucca]

@alisonjoseph @tay1orjones re-requesting revs 'cause I added a decent amount of code to handle TableHeaders independently of the body 😬

[laurenmrice]

Expansion story

• When I am on the Xs size, without text wrapping to multiple lines in rows, I notice the focus around the chevron icon is the correct size (32 x24), but when the text starts wrapping to multiple lines in rows, the focus box becomes larger in size. The focus box should always remain the same size (32 x 24) in both cases so that the chevron is in the center of the focus box.

• This also happens with the Lg size, but it's at first showing a very small focus box around the chevron icons when text lines do not wrap in rows, but once the text does start to wrap, the focus box is the correct size (32 x 32). It should always be 32 x 32 for both cases.

With Overflow Menu story

• Looks like there is some added padding to rows in the LG size than the XL size.

[francinelucca]

This should be good for re-rev @laurenmrice 🙏🏻

[laurenmrice]

Looks good to me! ✅

re-requested

[carbon-bot]

Hey there! v11.31.0 was just released that references this issue/PR.