Uh oh!
There was an error while loading. Please reload this page.
Update the default value of unsafeDisableTooltip to false and remove redundant props and wrappers - #4702
Conversation
…unncessary props and wrappers
🦋 Changeset detectedLatest commit: fd6bd57 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
unsafeDisableTooltip to false and remove redundant props and wrappersunsafeDisableTooltip to false and remove redundant props and wrappersunsafeDisableTooltip to false and remove redundant props and wrappers| expect(tooltipEl).not.toHaveAttribute(':popover-open') | ||
| }) | ||
| // second ESC closes the dialog and returns focus |
There was a problem hiding this comment.
This test is failing - Not sure how to address this. I am trying to await before pressing the second ESC but it doesn't seem to be working. Any suggestions, thoughts? 🤔
| }) | ||
| it('Returns focus to returnFocusRef on escape', async () => { | ||
| const {getByTestId, queryByTestId} = HTMLRender(<Component />) |
There was a problem hiding this comment.
I had an issue with this existing component. It was supposed to go back to the return ref after pressing the ESC twice (First closing the tooltip, second closing the dialog) but for some reason it didn't work, even though I waited for the previous keyboard event to be completed. This test wasn't about the tooltip itself; it is about the Dialog so I updated to use a different dialog but let me know if I am misinterpreting the issue I encountered.
unsafeDisableTooltip to false and remove redundant props and wrappersunsafeDisableTooltip to false and remove redundant props and wrappers| <ButtonGroup> | ||
| <IconButton icon={PlusIcon} aria-label="Add" /> | ||
| <IconButton icon={DashIcon} aria-label="Subtract" /> | ||
| {/* We can remove these unsafe props after we resolve https://github.com/primer/react/issues/4129 */} |
| import {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef' | ||
| import {XIcon} from '@primer/octicons-react' | ||
| // Dialog v1 |
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/332110 |
broccolinisoup
commented
Jul 9, 2024
Integration tests pass https://github.com/github/github/pull/332110 and tested on RL, everything looks good 🎉 |
Closes#2008
Related #4696
Note
We managed to complete the roll out at dotcom 🎉 (View the progress here (internal link only))
Changelog
Changed
unsafeDisableTooltipto falseunsafeDisableTooltip={true}a coupe of IconButton instances where it is not safe to enable tooltips.Removed
unsafeDisableTooltip={false}prop since it is the default value now.Rollout strategy
Testing & Reviewing
Merge checklist