See if maxWidth in TooltipBase can be done in CSS class instead of inline

[shendy-a8c]

Spun off from this discussion https://github.com/Automattic/woocommerce-payments/pull/8140#discussion_r1501976242, it might be better to set TooltipBase's max-width in CSS instead of inline which made it harder for 'child' components to override this max-width.

[htdat]

@shendy-a8c and @Automattic/helix - since you have the most context here, would you like to add this issue to your backlog?

We're adding this message after evaluating it in the Gamma weekly prioritization call.

[naman03malhotra]

IMO, we can include this as part of Helix backlog, but I will leave it upto @haszari to decide as I am still new to the team and would like to understand our acceptance criteria better 😄

[haszari]

We're happy to add this to our backlog, since we are heavy users of the tooltip component (and have some ideas to improve it!).

However I would be open to any other team picking this up too :)

Note at present, we wouldn't find this issue in our porter backlog search, since it doesn't have a relevant component/focus label (e.g. deposits, disputes, reporting etc). I'll manually add to our board so it doesn't get dropped.

[haszari]

An alternative here is to confirm that inline / css-in-js approach is required or recommended here, and document so there's no confusion.

[haszari]

Leaving this in Helix board as we're interested (and will ship more tooltips in payments widget project). Noting that this is not in our focus areas, other teams are welcome to pick it up.

[vbelolapotkov]

@haszari I'll set focus as deposits, since you are taking care of this issue.

[Jinksi]

I had a dive into this and attempted replacing the existing inline maxWidth with custom .scss overrides. It works, but I'm not sure it is a better solution.

TLDR with examples

I tried both methods and like method 1 better.

Method 1 (current): JSX maxWidth prop

<Tooltip maxWidth={'350px'} content="I am a long string that requires a larger max-width" />
<Tooltip maxWidth={ isTooltipContentLong ? '567px' : undefined } content="I am a long string that may require a larger max-width based on the boolean isTooltipContentLong" />
<Tooltip content="I am a normal tooltip that will inherit the default max-width" />

Method 2: CSS override method

<Tooltip id="tooltip-with-custom-id-1" content="I am a long string that requires a larger max-width" />
<Tooltip id={isTooltipContentLong ? "tooltip-with-custom-id-2" : undefined } content="I am a long string that may require a larger max-width based on the boolean isTooltipContentLong" />
<Tooltip content="I am a normal tooltip that will inherit the default max-width" />

#tooltip-with-custom-id-1 {
  max-width: 350px;
}

#tooltip-with-custom-id-2 {
  max-width: 567px;
}

---

Thoughts

inline which made it harder for 'child' components to override this max-width.

I might need some clarification of the exact pain-point/problem here.

Is it easier to override this max-width via CSS or JS?

In this case, I think it may be more clear to define the maxWidth alongside the tooltip content (in JSX), rather than require a unique class/ID and custom CSS.

E.g. in this file, we'd need to create an ID (or similar unique selector) for the 2 tooltips, since they share a classname, and write css to target each with a distinct max-width.

One tooltip also seems to require a custom max-width only when the balance is negative, so we'd need to pass this condition as a classname or other attribute.

I'm unsure if this is an easier or harder approach than passing a maxWidth prop.

The maxWidth prop is currently used 4 times in the codebase AFAICT

---

Lastly, here is an example with no max-width set that illustrates the need for a default tooltip max-width:

vs the desired design

[shendy-a8c]

I can think of these cons of using maxWidth prop in JS:

• We need to do 'prop drilling' (is that the correct term?).
• We cannot dynamically set max width by screen size. Although this might not be too important since admin pages are usually viewed on desktop and currently we use max width that will work both for desktop and mobile.
• I assume styling should be done in CSS and not in JS. Is that the correct best practice assumption? When styling needs to be done in JS, it makes sense when there is complicated computation needed in JS, which is not the case here.

[haszari]

My take (broadly aligned with @shendy-a8c):

• If we can avoid a prop, that will make our markup leaner and move more presentation concerns to css.
  • CSS can handle responsive adaptations.
• Prop drilling concern is real. HOWEVER, if we use less single-purpose components (wrappers for ToolTip) that would address that problem.
• Gentle pushback that we "would need an id or class" to use CSS. In most cases, we'd have an outer selector / class in a container that we can use.

For me it's a combination of 2 principles:

1. Styling should be in CSS unless there are computation needs that CSS can't handle (these are rare nowadays). Make our "software" lean and rely on browser rendering engine for high-performance styling.
2. Our components should be small, general-purpose, reusable components e.g. Link, ToolTip, CurrencyValue, Card etc.

I think this scenario is complicated by the fact that we have multiple ToolTip wrapper components; I suspect we could use a generic ToolTip directly in most cases.

[haszari]

Marking as low but if this annoys me again I may bump to medium.

Next steps might be:

• Review all our tooltips, work out why we need to customise the width.
• Understand the semantic use cases for those customisations.
• Figure out if we can support that in a base component with semantic props.

I think the code should not have so much control over presentation, it should just say "I want to show a tooltip, here's the content". Then we wouldn't have issues trying to override the specifics in different use-cases of tooltip.

This is complicated by the fact that we have lots of custom wrappers for Tooltip which IMO are not adding value, and make the prop-drilling problem more apparent.