Add controlled prop to open the Tooltip

[aaronccasanova]

WHAT is this pull request doing?

This PR introduces a controlled component prop named open to the Tooltip. This feature is being added to support upcoming Common Actions guidance for Copy to Clipboard buttons.

Currently, the Tooltip provides an active prop which only sets the initial open state. Note: The new open prop takes precedence over active. I suggest we rename active to defaultOpen in a future major release for consistency with other (un)controlled component patterns (e.g. <input />, value, and defaultValue) and help disambiguate the props.

UPDATE: Per the discussion here, we have opt'd to include the proposed defaultProp and deprecate the active prop as part of this PR e.g.

• Added a new defaultOpen prop
• Deprecated the active prop
• Special cased the existing active prop behavior
• Added new test cases for defaultOpen
• Updated existing test cases to use defaultOpen
• Added uncontrolled Tooltip examples to Storybook
• Updated existing Storybook entries to use defaultOpen

Example setting open={true} for the duration of the copied status timeout:

Screen.Recording.2024-03-01.at.3.11.05.PM.mov

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

• Tested a snapshot
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated the component's README.md with documentation changes
• Tophatted documentation changes in the style guide

[chloerice]

Should we deprecate active and add the defaultOpen prop now in anticipation of v13?

[aaronccasanova]

Good idea 👍 I'll push up a commit shortly.

[aaronccasanova]

Actually, I tentatively think we should handle this in a follow up. I attempted the rename here. However, the active props seems to have one controlled prop-like behavior in that active={false} does force the closed state... We can special case the existing active behavior with conditionals for backward compatibility or just wait till Polaris v13. Thoughts?

[sam-b-rose]

Even though defaultOpen doesn't behave exactly like active, I think it is worth deprecating active here and adding defaultOpen. I think active is currently an odd prop mix of part-controlled uncontrolled.

I think it would be better to leave the active behavior as-is until and remove it on the next major.

Alternatively, we introduce a defaultActive prop and "fix" active to behave like a fully controlled prop (state is external). Though, this strategy would also be a breaking change which would delay Common Action progress

[aaronccasanova]

I think it would be better to leave the active behavior as-is until and remove it on the next major.

Agreed 👍

[aaronccasanova]

Update:

• Added a new defaultOpen prop
• Deprecated the active prop
• Special cased the existing active prop behavior
• Added new test cases for defaultOpen
• Updated existing test cases to use defaultOpen
• Added uncontrolled Tooltip examples to Storybook
• Updated existing Storybook entries to use defaultOpen

[chloerice]

Without adding onEnter/onLeave handlers for consumers to control active state within Tooltip itself, how does open get controlled? I see that it's controlled externally by some other interactive activator element in the story, but the tooltip's presence being controlled by another element doesn't make sense 🤔

[aaronccasanova]

That's actually my current use case in the Copy to Clipboard recipe (e.g. the copy button triggers a timeout which sets open={true} only). However, I agree with the sentiment it is more conventional to show controlled component examples with the state and handlers 👍 Updated! 7619a24

[sam-b-rose]

Even though defaultOpen doesn't behave exactly like active, I think it is worth deprecating active here and adding defaultOpen. I think active is currently an odd prop mix of part-controlled uncontrolled.

I think it would be better to leave the active behavior as-is until and remove it on the next major.

Alternatively, we introduce a defaultActive prop and "fix" active to behave like a fully controlled prop (state is external). Though, this strategy would also be a breaking change which would delay Common Action progress

[aaronccasanova]

cc @chloerice @laurkim @sam-b-rose requested new reviews as a LOT has changed per the discussion here

[chloerice]

Works great @aaronccasanova, can't wait to be rid of that active prop 🙌🏽 🚀

[aaronccasanova]

/snapit

[github-actions]

🫰✨ Thanks @aaronccasanova! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]