From feb69db90243f39d073b9e6bb9cb395e94a0a71a Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 1 Aug 2022 22:26:03 +0200 Subject: [PATCH 1/6] Popover: fix arrow logic, use SVG to render triangle --- packages/components/src/popover/index.js | 61 ++++++++++++++------- packages/components/src/popover/style.scss | 63 +++++++++++++++++++--- 2 files changed, 98 insertions(+), 26 deletions(-) diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js index 0b9e3f0a981772..19f99342b2eb81 100644 --- a/packages/components/src/popover/index.js +++ b/packages/components/src/popover/index.js @@ -32,6 +32,7 @@ import { } from '@wordpress/compose'; import { close } from '@wordpress/icons'; import deprecated from '@wordpress/deprecated'; +import { Path, SVG } from '@wordpress/primitives'; /** * Internal dependencies @@ -48,6 +49,30 @@ import { getAnimateClassName } from '../animate'; */ const SLOT_NAME = 'Popover'; +// An SVG displaying a triangle facing down, filled with a solid +// color and bordered in such a way to create an arrow-like effect. +// Keeping the SVG's viewbox squared simplify the arrow positioning +// calculations. +const ArrowTriangle = ( props ) => ( + + + + +); + const slotNameContext = createContext(); const positionToPlacement = ( position ) => { @@ -266,12 +291,7 @@ const Popover = ( placement: usedPlacement, middleware: middlewares, } ); - const staticSide = { - top: 'bottom', - right: 'left', - bottom: 'top', - left: 'right', - }[ placementData.split( '-' )[ 0 ] ]; + const mergedRefs = useMergeRefs( [ floating, dialogRef, ref ] ); // Updates references @@ -407,22 +427,25 @@ const Popover = (
{ children }
{ hasArrow && (
+ > + +
) } ); diff --git a/packages/components/src/popover/style.scss b/packages/components/src/popover/style.scss index 6002139c2c8566..f915e9376e906c 100644 --- a/packages/components/src/popover/style.scss +++ b/packages/components/src/popover/style.scss @@ -60,12 +60,61 @@ .components-popover__arrow { position: absolute; - background: $gray-400; - width: 8px; - height: 8px; - transform: rotate(45deg); - z-index: -1; - .is-alternate & { - background: $gray-900; + width: var(--wp-popover-arrow-size); + height: var(--wp-popover-arrow-size); + pointer-events: none; + + // Thin line that helps to make sure that the underlying + // popover__content's outline is fully overlapped by the + // arrow + &::before { + content: ""; + position: absolute; + top: -1px; + left: 0; + height: 2px; + width: 100%; + background-color: $white; + } + + // Position and rotate the arrow depending on the popover's placement. + // The `!important' is necessary to override the inline styles. + &--top { + bottom: calc(-1 * var(--wp-popover-arrow-size)) !important; + transform: rotate(0); + } + &--right { + left: calc(-1 * var(--wp-popover-arrow-size)) !important; + transform: rotate(90deg); + } + &--bottom { + top: calc(-1 * var(--wp-popover-arrow-size)) !important; + transform: rotate(180deg); + } + &--left { + right: calc(-1 * var(--wp-popover-arrow-size)) !important; + transform: rotate(-90deg); + } +} + +.components-popover__triangle { + fill: transparent; + position: absolute; + height: 100%; + width: 100%; + stroke-width: $border-width; + + // Same as popover content + &-bg { + fill: $white; + stroke: $white; + } + + &-border { + stroke: $gray-400; + } + + .is-alternate &-border { + stroke: $gray-900; } } From b8976c15984aa50fb442e81310874f62696f7065 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 1 Aug 2022 23:26:02 +0200 Subject: [PATCH 2/6] CHANGELOG --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 4196d8be8f5524..2fbc3447917a1e 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -8,6 +8,7 @@ - `BorderControl`: Ensure box-sizing is reset for the control ([#42754](https://github.com/WordPress/gutenberg/pull/42754)). - `InputControl`: Fix acceptance of falsy values in controlled updates ([#42484](https://github.com/WordPress/gutenberg/pull/42484/)). - `Tooltip (Experimental)`, `CustomSelectControl`, `TimePicker`: Add missing font-size styles which were necessary in non-WordPress contexts ([#42844](https://github.com/WordPress/gutenberg/pull/42844/)). +- `Popover`: fix arrow placement and design ([#42874](https://github.com/WordPress/gutenberg/pull/42874/)). ### Enhancements From 6bec73f0f65d0503599009251657dbc53f4e05c3 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 1 Aug 2022 23:45:45 +0200 Subject: [PATCH 3/6] Add RTL support --- packages/components/src/popover/style.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/components/src/popover/style.scss b/packages/components/src/popover/style.scss index f915e9376e906c..d18c053acab264 100644 --- a/packages/components/src/popover/style.scss +++ b/packages/components/src/popover/style.scss @@ -84,6 +84,7 @@ transform: rotate(0); } &--right { + /*rtl:begin:ignore*/ left: calc(-1 * var(--wp-popover-arrow-size)) !important; transform: rotate(90deg); } @@ -92,8 +93,10 @@ transform: rotate(180deg); } &--left { + /*rtl:begin:ignore*/ right: calc(-1 * var(--wp-popover-arrow-size)) !important; transform: rotate(-90deg); + /*rtl:end:ignore*/ } } From afa77d63107a8f9d1b8ab276e13fca83569365b5 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 2 Aug 2022 09:39:33 +0200 Subject: [PATCH 4/6] Use modifier classes instead of traditional BEM --- packages/components/src/popover/index.js | 4 +--- packages/components/src/popover/style.scss | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js index 19f99342b2eb81..9424eb2f0268fe 100644 --- a/packages/components/src/popover/index.js +++ b/packages/components/src/popover/index.js @@ -430,9 +430,7 @@ const Popover = ( ref={ arrowRef } className={ [ 'components-popover__arrow', - `components-popover__arrow--${ - placementData.split( '-' )[ 0 ] - }`, + `is-${ placementData.split( '-' )[ 0 ] }`, ].join( ' ' ) } style={ { left: Number.isFinite( arrowData?.x ) diff --git a/packages/components/src/popover/style.scss b/packages/components/src/popover/style.scss index d18c053acab264..65dc12b3e77502 100644 --- a/packages/components/src/popover/style.scss +++ b/packages/components/src/popover/style.scss @@ -79,20 +79,20 @@ // Position and rotate the arrow depending on the popover's placement. // The `!important' is necessary to override the inline styles. - &--top { + &.is-top { bottom: calc(-1 * var(--wp-popover-arrow-size)) !important; transform: rotate(0); } - &--right { + &.is-right { /*rtl:begin:ignore*/ left: calc(-1 * var(--wp-popover-arrow-size)) !important; transform: rotate(90deg); } - &--bottom { + &.is-bottom { top: calc(-1 * var(--wp-popover-arrow-size)) !important; transform: rotate(180deg); } - &--left { + &.is-left { /*rtl:begin:ignore*/ right: calc(-1 * var(--wp-popover-arrow-size)) !important; transform: rotate(-90deg); From 5c62523a6cbe6e47382067c3a9ec3693ebb36792 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 2 Aug 2022 09:48:02 +0200 Subject: [PATCH 5/6] Use full classnames for triangle SVG --- packages/components/src/popover/style.scss | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/components/src/popover/style.scss b/packages/components/src/popover/style.scss index 65dc12b3e77502..57f2c246498553 100644 --- a/packages/components/src/popover/style.scss +++ b/packages/components/src/popover/style.scss @@ -101,23 +101,23 @@ } .components-popover__triangle { - fill: transparent; position: absolute; height: 100%; width: 100%; - stroke-width: $border-width; +} - // Same as popover content - &-bg { - fill: $white; - stroke: $white; - } +.components-popover__triangle-bg { + // Fill color is the same as the .components-popover__content's background + fill: $white; +} - &-border { - stroke: $gray-400; - } +.components-popover__triangle-border { + // Stroke colors are the same as the .components-popover__content's outline + fill: transparent; + stroke-width: $border-width; + stroke: $gray-400; - .is-alternate &-border { + .is-alternate & { stroke: $gray-900; } } From 04e37ba4f1f1ebaba4295cfd4e993f67727543f6 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 2 Aug 2022 10:30:44 +0200 Subject: [PATCH 6/6] Use SCSS variable instead of CSS custom property --- packages/components/src/popover/index.js | 1 - packages/components/src/popover/style.scss | 14 ++++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js index 9424eb2f0268fe..c3f56c93895fe5 100644 --- a/packages/components/src/popover/index.js +++ b/packages/components/src/popover/index.js @@ -439,7 +439,6 @@ const Popover = ( top: Number.isFinite( arrowData?.y ) ? `${ arrowData.y }px` : '', - '--wp-popover-arrow-size': '14px', } } > diff --git a/packages/components/src/popover/style.scss b/packages/components/src/popover/style.scss index 57f2c246498553..00639f67216781 100644 --- a/packages/components/src/popover/style.scss +++ b/packages/components/src/popover/style.scss @@ -1,3 +1,5 @@ +$arrow-triangle-base-size: 14px; + .components-popover { z-index: z-index(".components-popover"); @@ -60,8 +62,8 @@ .components-popover__arrow { position: absolute; - width: var(--wp-popover-arrow-size); - height: var(--wp-popover-arrow-size); + width: $arrow-triangle-base-size; + height: $arrow-triangle-base-size; pointer-events: none; // Thin line that helps to make sure that the underlying @@ -80,21 +82,21 @@ // Position and rotate the arrow depending on the popover's placement. // The `!important' is necessary to override the inline styles. &.is-top { - bottom: calc(-1 * var(--wp-popover-arrow-size)) !important; + bottom: -1 * $arrow-triangle-base-size !important; transform: rotate(0); } &.is-right { /*rtl:begin:ignore*/ - left: calc(-1 * var(--wp-popover-arrow-size)) !important; + left: -1 * $arrow-triangle-base-size !important; transform: rotate(90deg); } &.is-bottom { - top: calc(-1 * var(--wp-popover-arrow-size)) !important; + top: -1 * $arrow-triangle-base-size !important; transform: rotate(180deg); } &.is-left { /*rtl:begin:ignore*/ - right: calc(-1 * var(--wp-popover-arrow-size)) !important; + right: -1 * $arrow-triangle-base-size !important; transform: rotate(-90deg); /*rtl:end:ignore*/ }