From 3e42bc343fe4f58b69e16d88d7e4ac885ba0eaeb Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 19 Apr 2023 13:42:13 +1000 Subject: [PATCH 1/7] Draggable: Allow elementId based elements to be attached to the ownerDocument body --- packages/components/src/draggable/README.md | 9 ++- packages/components/src/draggable/index.tsx | 7 +- .../src/draggable/stories/index.tsx | 69 ++++++++++++++++++- .../src/draggable/stories/style.css | 12 ++++ packages/components/src/draggable/types.ts | 7 ++ 5 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 packages/components/src/draggable/stories/style.css diff --git a/packages/components/src/draggable/README.md b/packages/components/src/draggable/README.md index f83ab93dd3b3c..e224791dd7eeb 100644 --- a/packages/components/src/draggable/README.md +++ b/packages/components/src/draggable/README.md @@ -8,9 +8,16 @@ Note that the drag handle needs to declare the `draggable="true"` property and b The component accepts the following props: +### `attachToElementWrapper`: `boolean` + +Whether to attach the cloned element to the element's wrapper when using `elementId`. Setting to false will attach the cloned element to the `ownerDocument` body. + +- Required: No +- Default: `true` + ### `elementId`: `string` -The HTML id of the element to clone on drag +The HTML id of the element to clone on drag. - Required: Yes diff --git a/packages/components/src/draggable/index.tsx b/packages/components/src/draggable/index.tsx index 2b1244c9b1e66..ea1f81052481f 100644 --- a/packages/components/src/draggable/index.tsx +++ b/packages/components/src/draggable/index.tsx @@ -63,6 +63,7 @@ export function Draggable( { onDragStart, onDragOver, onDragEnd, + attachElementToWrapper = true, cloneClassname, elementId, transferData, @@ -173,7 +174,11 @@ export function Draggable( { cloneWrapper.appendChild( clone ); // Inject the cloneWrapper into the DOM. - elementWrapper?.appendChild( cloneWrapper ); + if ( attachElementToWrapper ) { + elementWrapper?.appendChild( cloneWrapper ); + } else { + ownerDocument.body.appendChild( cloneWrapper ); + } } // Mark the current cursor coordinates. diff --git a/packages/components/src/draggable/stories/index.tsx b/packages/components/src/draggable/stories/index.tsx index 0fa16f063bb8f..960fbb1d82e14 100644 --- a/packages/components/src/draggable/stories/index.tsx +++ b/packages/components/src/draggable/stories/index.tsx @@ -14,6 +14,7 @@ import { Icon, more } from '@wordpress/icons'; * Internal dependencies */ import Draggable from '..'; +import './style.css'; const meta: ComponentMeta< typeof Draggable > = { component: Draggable, @@ -38,7 +39,7 @@ const DefaultTemplate: ComponentStory< typeof Draggable > = ( args ) => {

Is Dragging? { isDragging ? 'Yes' : 'No!' }

@@ -81,3 +82,69 @@ export const Default: ComponentStory< typeof Draggable > = DefaultTemplate.bind( {} ); Default.args = {}; + +const AttachElementIdTemplate: ComponentStory< typeof Draggable > = ( + args +) => { + const [ isDragging, setDragging ] = useState( false ); + + const label = args.attachElementToWrapper + ? 'When dragging, the element is attached to the wrapper and will receive styling rules from its parent' + : 'When dragging, the element is attached to the body and will not receive styling rules from its original parent'; + + // Allow for the use of ID in the example. + return ( +
+

Is Dragging? { isDragging ? 'Yes' : 'No!' }

+

{ label }

+
+ + { ( { onDraggableStart, onDraggableEnd } ) => { + const handleOnDragStart = ( event: DragEvent ) => { + setDragging( true ); + onDraggableStart( event ); + }; + const handleOnDragEnd = ( event: DragEvent ) => { + setDragging( false ); + onDraggableEnd( event ); + }; + + return ( +
+ +
+ ); + } } + +
+
+ ); +}; + +export const AttachElementToWrapper: ComponentStory< typeof Draggable > = + AttachElementIdTemplate.bind( {} ); +AttachElementToWrapper.args = { + attachElementToWrapper: true, +}; + +export const AttachElementToBody: ComponentStory< typeof Draggable > = + AttachElementIdTemplate.bind( {} ); +AttachElementToBody.args = { + attachElementToWrapper: false, +}; diff --git a/packages/components/src/draggable/stories/style.css b/packages/components/src/draggable/stories/style.css new file mode 100644 index 0000000000000..e939c2f025f09 --- /dev/null +++ b/packages/components/src/draggable/stories/style.css @@ -0,0 +1,12 @@ +/* .draggable-storybook-wrapper-element { + background-color: #99e7fa; + padding: 1em; +} */ + +.draggable-storybook-wrapper-element .draggable-storybook-element { + background-color: #99faa4; +} + +.draggable-storybook-element { + background-color: #fa99c8; +} diff --git a/packages/components/src/draggable/types.ts b/packages/components/src/draggable/types.ts index 665378a71ea99..b5096b9c7f562 100644 --- a/packages/components/src/draggable/types.ts +++ b/packages/components/src/draggable/types.ts @@ -17,6 +17,13 @@ export type DraggableProps = { */ onDraggableEnd: ( event: DragEvent ) => void; } ) => JSX.Element | null; + /** + * Whether to attach the cloned element to the element's wrapper when using `elementId`. + * Setting to false will attach the cloned element to the `ownerDocument` body. + * + *@default true + */ + attachElementToWrapper?: boolean; /** * Classname for the cloned element. */ From 4a4759646f49405cedb0e2507b63ffa15698b74f Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 19 Apr 2023 13:53:48 +1000 Subject: [PATCH 2/7] Update changelog --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 2566d6c4dc2c7..1693f4960b6d6 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -4,6 +4,7 @@ ### Enhancements +- `Draggable`: Add `attachToElementWrapper` prop to allow elementId based elements to be attached to the ownerDocument body ([#49911](https://github.com/WordPress/gutenberg/pull/49911)) - `TreeGrid`: Modify keyboard navigation code to use a data-expanded attribute if aria-expanded is to be controlled outside of the TreeGrid component ([#48461](https://github.com/WordPress/gutenberg/pull/48461)). - `Modal`: Equalize internal spacing ([#49890](https://github.com/WordPress/gutenberg/pull/49890)). - `Modal`: Increased border radius ([#49870](https://github.com/WordPress/gutenberg/pull/49870)). From 55b12123ba6b60c54398a1bca4b5e236ca32c715 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 19 Apr 2023 14:19:03 +1000 Subject: [PATCH 3/7] Remove comment --- packages/components/src/draggable/stories/style.css | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/components/src/draggable/stories/style.css b/packages/components/src/draggable/stories/style.css index e939c2f025f09..00a05dd631961 100644 --- a/packages/components/src/draggable/stories/style.css +++ b/packages/components/src/draggable/stories/style.css @@ -1,8 +1,3 @@ -/* .draggable-storybook-wrapper-element { - background-color: #99e7fa; - padding: 1em; -} */ - .draggable-storybook-wrapper-element .draggable-storybook-element { background-color: #99faa4; } From 15b869c0d9aa708e5dea55c63f6475d7fad11807 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 19 Apr 2023 14:19:48 +1000 Subject: [PATCH 4/7] Add missing fullstop in changelog --- packages/components/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 1693f4960b6d6..9d3682619480c 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -4,7 +4,7 @@ ### Enhancements -- `Draggable`: Add `attachToElementWrapper` prop to allow elementId based elements to be attached to the ownerDocument body ([#49911](https://github.com/WordPress/gutenberg/pull/49911)) +- `Draggable`: Add `attachToElementWrapper` prop to allow elementId based elements to be attached to the ownerDocument body ([#49911](https://github.com/WordPress/gutenberg/pull/49911)). - `TreeGrid`: Modify keyboard navigation code to use a data-expanded attribute if aria-expanded is to be controlled outside of the TreeGrid component ([#48461](https://github.com/WordPress/gutenberg/pull/48461)). - `Modal`: Equalize internal spacing ([#49890](https://github.com/WordPress/gutenberg/pull/49890)). - `Modal`: Increased border radius ([#49870](https://github.com/WordPress/gutenberg/pull/49870)). From 8a30984edca52d11e2e4c0466a45d05fb6135105 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Thu, 20 Apr 2023 14:54:53 +1000 Subject: [PATCH 5/7] Switch to appendToOwnerDocument, simplify stories --- packages/components/src/draggable/README.md | 6 +- packages/components/src/draggable/index.tsx | 8 +- .../src/draggable/stories/index.tsx | 165 +++++++----------- .../src/draggable/stories/style.css | 7 - packages/components/src/draggable/types.ts | 8 +- 5 files changed, 78 insertions(+), 116 deletions(-) delete mode 100644 packages/components/src/draggable/stories/style.css diff --git a/packages/components/src/draggable/README.md b/packages/components/src/draggable/README.md index e224791dd7eeb..58ffa2c6a6d0d 100644 --- a/packages/components/src/draggable/README.md +++ b/packages/components/src/draggable/README.md @@ -8,12 +8,12 @@ Note that the drag handle needs to declare the `draggable="true"` property and b The component accepts the following props: -### `attachToElementWrapper`: `boolean` +### `appendToOwnerDocument`: `boolean` -Whether to attach the cloned element to the element's wrapper when using `elementId`. Setting to false will attach the cloned element to the `ownerDocument` body. +Whether to append the cloned element to the `ownerDocument` body. By default, elements sourced by id are appended to the element's wrapper. - Required: No -- Default: `true` +- Default: `false` ### `elementId`: `string` diff --git a/packages/components/src/draggable/index.tsx b/packages/components/src/draggable/index.tsx index ea1f81052481f..1f839332b66c0 100644 --- a/packages/components/src/draggable/index.tsx +++ b/packages/components/src/draggable/index.tsx @@ -63,7 +63,7 @@ export function Draggable( { onDragStart, onDragOver, onDragEnd, - attachElementToWrapper = true, + appendToOwnerDocument = false, cloneClassname, elementId, transferData, @@ -174,10 +174,10 @@ export function Draggable( { cloneWrapper.appendChild( clone ); // Inject the cloneWrapper into the DOM. - if ( attachElementToWrapper ) { - elementWrapper?.appendChild( cloneWrapper ); - } else { + if ( appendToOwnerDocument ) { ownerDocument.body.appendChild( cloneWrapper ); + } else { + elementWrapper?.appendChild( cloneWrapper ); } } diff --git a/packages/components/src/draggable/stories/index.tsx b/packages/components/src/draggable/stories/index.tsx index 960fbb1d82e14..ad94802feb93a 100644 --- a/packages/components/src/draggable/stories/index.tsx +++ b/packages/components/src/draggable/stories/index.tsx @@ -7,6 +7,7 @@ import type { DragEvent } from 'react'; /** * WordPress dependencies */ +import { useInstanceId } from '@wordpress/compose'; import { useState } from '@wordpress/element'; import { Icon, more } from '@wordpress/icons'; @@ -14,7 +15,6 @@ import { Icon, more } from '@wordpress/icons'; * Internal dependencies */ import Draggable from '..'; -import './style.css'; const meta: ComponentMeta< typeof Draggable > = { component: Draggable, @@ -33,46 +33,68 @@ export default meta; const DefaultTemplate: ComponentStory< typeof Draggable > = ( args ) => { const [ isDragging, setDragging ] = useState( false ); + const instanceId = useInstanceId( DefaultTemplate ); // Allow for the use of ID in the example. return (
-

Is Dragging? { isDragging ? 'Yes' : 'No!' }

+

+ Is Dragging? { isDragging ? 'Yes' : 'No!' } +

- - { ( { onDraggableStart, onDraggableEnd } ) => { - const handleOnDragStart = ( event: DragEvent ) => { - setDragging( true ); - onDraggableStart( event ); - }; - const handleOnDragEnd = ( event: DragEvent ) => { - setDragging( false ); - onDraggableEnd( event ); - }; - - return ( -
- -
- ); +
+ > + + { ( { onDraggableStart, onDraggableEnd } ) => { + const handleOnDragStart = ( event: DragEvent ) => { + setDragging( true ); + onDraggableStart( event ); + }; + const handleOnDragEnd = ( event: DragEvent ) => { + setDragging( false ); + onDraggableEnd( event ); + }; + + return ( +
+ +
+ ); + } } + +
); @@ -83,68 +105,15 @@ export const Default: ComponentStory< typeof Draggable > = DefaultTemplate.bind( ); Default.args = {}; -const AttachElementIdTemplate: ComponentStory< typeof Draggable > = ( - args -) => { - const [ isDragging, setDragging ] = useState( false ); - - const label = args.attachElementToWrapper - ? 'When dragging, the element is attached to the wrapper and will receive styling rules from its parent' - : 'When dragging, the element is attached to the body and will not receive styling rules from its original parent'; - - // Allow for the use of ID in the example. - return ( -
-

Is Dragging? { isDragging ? 'Yes' : 'No!' }

-

{ label }

-
- - { ( { onDraggableStart, onDraggableEnd } ) => { - const handleOnDragStart = ( event: DragEvent ) => { - setDragging( true ); - onDraggableStart( event ); - }; - const handleOnDragEnd = ( event: DragEvent ) => { - setDragging( false ); - onDraggableEnd( event ); - }; - - return ( -
- -
- ); - } } - -
-
- ); -}; - -export const AttachElementToWrapper: ComponentStory< typeof Draggable > = - AttachElementIdTemplate.bind( {} ); -AttachElementToWrapper.args = { - attachElementToWrapper: true, -}; - -export const AttachElementToBody: ComponentStory< typeof Draggable > = - AttachElementIdTemplate.bind( {} ); -AttachElementToBody.args = { - attachElementToWrapper: false, +/** + * `appendToOwnerDocument` is used to append the element being dragged to the body of the owner document. + * + * This is useful when the element being dragged should not receive styles from its parent. + * For example, when the element's parent sets a `z-index` value that would cause the dragged + * element to be rendered behind other elements. + */ +export const AppendElementToOwnerDocument: ComponentStory< typeof Draggable > = + DefaultTemplate.bind( {} ); +AppendElementToOwnerDocument.args = { + appendToOwnerDocument: true, }; diff --git a/packages/components/src/draggable/stories/style.css b/packages/components/src/draggable/stories/style.css deleted file mode 100644 index 00a05dd631961..0000000000000 --- a/packages/components/src/draggable/stories/style.css +++ /dev/null @@ -1,7 +0,0 @@ -.draggable-storybook-wrapper-element .draggable-storybook-element { - background-color: #99faa4; -} - -.draggable-storybook-element { - background-color: #fa99c8; -} diff --git a/packages/components/src/draggable/types.ts b/packages/components/src/draggable/types.ts index b5096b9c7f562..d3d5548c84a0d 100644 --- a/packages/components/src/draggable/types.ts +++ b/packages/components/src/draggable/types.ts @@ -18,12 +18,12 @@ export type DraggableProps = { onDraggableEnd: ( event: DragEvent ) => void; } ) => JSX.Element | null; /** - * Whether to attach the cloned element to the element's wrapper when using `elementId`. - * Setting to false will attach the cloned element to the `ownerDocument` body. + * Whether to append the cloned element to the `ownerDocument` body. + * By default, elements sourced by id are appended to the element's wrapper. * - *@default true + *@default false */ - attachElementToWrapper?: boolean; + appendToOwnerDocument?: boolean; /** * Classname for the cloned element. */ From f043e9a8e8d1fc7b7ff00125fe9295ebb9a9d5a9 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Thu, 20 Apr 2023 15:07:56 +1000 Subject: [PATCH 6/7] Update changelog entry --- packages/components/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 9d3682619480c..2743166026a3a 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -4,7 +4,7 @@ ### Enhancements -- `Draggable`: Add `attachToElementWrapper` prop to allow elementId based elements to be attached to the ownerDocument body ([#49911](https://github.com/WordPress/gutenberg/pull/49911)). +- `Draggable`: Add `appendToOwnerDocument` prop to allow elementId based elements to be attached to the ownerDocument body ([#49911](https://github.com/WordPress/gutenberg/pull/49911)). - `TreeGrid`: Modify keyboard navigation code to use a data-expanded attribute if aria-expanded is to be controlled outside of the TreeGrid component ([#48461](https://github.com/WordPress/gutenberg/pull/48461)). - `Modal`: Equalize internal spacing ([#49890](https://github.com/WordPress/gutenberg/pull/49890)). - `Modal`: Increased border radius ([#49870](https://github.com/WordPress/gutenberg/pull/49870)). From 4a90cff74cb1ad726eae0d6501a2e9a63f84d944 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 21 Apr 2023 08:21:44 +1000 Subject: [PATCH 7/7] Fix formatting issue Co-authored-by: Lena Morita --- packages/components/src/draggable/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/draggable/types.ts b/packages/components/src/draggable/types.ts index d3d5548c84a0d..5242ecf7d67eb 100644 --- a/packages/components/src/draggable/types.ts +++ b/packages/components/src/draggable/types.ts @@ -21,7 +21,7 @@ export type DraggableProps = { * Whether to append the cloned element to the `ownerDocument` body. * By default, elements sourced by id are appended to the element's wrapper. * - *@default false + * @default false */ appendToOwnerDocument?: boolean; /**