Skip to content

Remove unused components from ui/ #54573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brookewp merged 6 commits into from
Sep 21, 2023
Merged

Remove unused components from ui/ #54573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9658f1d
Replace ui tooltip
brookewp Sep 18, 2023
3f9a205
Remove ui tooltip
brookewp Sep 18, 2023
143d1a6
Remove ui shortcut
brookewp Sep 18, 2023
bdf2226
Update changelog
brookewp Sep 18, 2023
2d39b03
Remove related unused code
brookewp Sep 20, 2023
7d287e4
Reverted unrelated prop change
brookewp Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
- `Placeholder`: Improved DOM structure and screen reader announcements ([#45801](https://github.com/WordPress/gutenberg/pull/45801)).
- `DateTimePicker`: fix onChange callback check so that it also works inside iframes ([#54669](https://github.com/WordPress/gutenberg/pull/54669)).

### Internal

- `Tooltip`, `Shortcut`: Remove unused `ui/` components from the codebase ([#54573](https://github.com/WordPress/gutenberg/pull/54573))

## 25.8.0 (2023-09-20)

### Enhancements
Expand Down
14 changes: 5 additions & 9 deletions packages/components/src/color-picker/color-copy-button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import { __ } from '@wordpress/i18n';
* Internal dependencies
*/
import { CopyButton } from './styles';
import { Text } from '../text';
import { Tooltip } from '../ui/tooltip';
import Tooltip from '../tooltip';

import type { ColorCopyButtonProps } from './types';

Expand Down Expand Up @@ -56,14 +55,11 @@ export const ColorCopyButton = ( props: ColorCopyButtonProps ) => {

return (
<Tooltip
content={
<Text color="white">
{ copiedColor === color.toHex()
? __( 'Copied!' )
: __( 'Copy' ) }
</Text>
delay={ 0 }
hideOnClick={ false }
text={
copiedColor === color.toHex() ? __( 'Copied!' ) : __( 'Copy' )
}
placement="bottom"
>
<CopyButton
isSmall
Expand Down
1 change: 0 additions & 1 deletion packages/components/src/tooltip/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ function Tooltip( props: TooltipProps ) {
return (
<>
<Ariakit.TooltipAnchor
onBlur={ tooltipStore.hide }
Copy link
Member

@tyxla tyxla Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please elaborate on this change?

Copy link
Contributor Author

@brookewp brookewp Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you for asking! I meant to leave a comment here when I opened the PR.

There was a conflict between the onBlur added to the Tooltip anchor and the function useCopyToClipboard in the ColorPicker component:

gutenberg/packages/components/src/color-picker/color-copy-button.tsx

Line 21 in 70f787b

const copyRef = useCopyToClipboard< HTMLDivElement >(

In this function, the focus is reset, resulting in the tooltip hiding on copy button click before displaying the 'copied' text:

gutenberg/packages/compose/src/hooks/use-copy-to-clipboard/index.js

Lines 52 to 59 in 23bb930

clipboard.on( 'success', ( { clearSelection } ) => {
// Clearing selection will move focus back to the triggering
// button, ensuring that it is not reset to the body, and
// further that it is kept within the rendered node.
clearSelection();
// Handle ClipboardJS focus bug, see
// https://github.com/zenorocha/clipboard.js/issues/680
node.focus();

Originally, we added the onBlur to the tooltip to replicate the legacy behavior where the tooltip is hidden when leaving the document. This was discovered through failing tests; but now, the tests pass without it. The utility function was added after, so it's likely the failure was related to leaky tests instead.

I discussed removing this or adding it as a prop with @ciampo, and we decided to remove it for now. And if we find a use-case where it makes sense to have it, we could add it as an optional prop.

Copy link
Member

@tyxla tyxla Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, thanks 👍

Copy link
Contributor

@ciampo ciampo Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to consider adding a prop to customize this behaviour, since this change is related to a regression #57206

onClick={ hideOnClick ? tooltipStore.hide : undefined }
store={ tooltipStore }
render={ isOnlyChild ? children : undefined }
Expand Down
62 changes: 0 additions & 62 deletions packages/components/src/ui/shortcut/component.tsx
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions packages/components/src/ui/shortcut/index.ts
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions packages/components/src/ui/shortcut/test/__snapshots__/index.js.snap
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions packages/components/src/ui/shortcut/test/index.js
View file
Open in desktop

This file was deleted.

21 changes: 0 additions & 21 deletions packages/components/src/ui/tooltip/README.md
View file
Open in desktop

This file was deleted.

102 changes: 0 additions & 102 deletions packages/components/src/ui/tooltip/component.js
View file
Open in desktop

This file was deleted.

45 changes: 0 additions & 45 deletions packages/components/src/ui/tooltip/content.js
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions packages/components/src/ui/tooltip/context.js
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions packages/components/src/ui/tooltip/index.js
View file
Open in desktop

This file was deleted.

26 changes: 0 additions & 26 deletions packages/components/src/ui/tooltip/stories/index.story.js
View file
Open in desktop

This file was deleted.

Loading