Skip to content
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.

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

fix: refine arrowStyles CSS #1812

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions packages/dropdowns.legacy/src/styled/menu/StyledMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ export const StyledMenu = styled.ul.attrs<IStyledMenuProps>(props => ({
${props =>
props.hasArrow &&
arrowStyles(getArrowPosition(props.placement), {
size: `${props.theme.space.base * 2}px`,
inset: '2px',
inset: '1.5px',
animationModifier: props.isAnimated ? '.is-animated' : undefined
})};

Expand Down
3 changes: 1 addition & 2 deletions packages/dropdowns/src/views/menu/StyledMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ export const StyledMenu = styled(StyledListbox).attrs({
${props =>
props.arrowPosition &&
arrowStyles(props.arrowPosition, {
size: `${props.theme.space.base * 2}px`,
inset: '2px',
inset: '1.5px',
animationModifier: '[data-garden-animate-arrow="true"]'
})};

Expand Down
3 changes: 1 addition & 2 deletions packages/modals/src/styled/StyledTooltipModal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ export const StyledTooltipModal = styled.div.attrs<IStyledTooltipModalProps>(pro

${props => {
const computedArrowStyles = arrowStyles(getArrowPosition(props.theme, props.placement), {
size: `${props.theme.space.base * 2}px`,
inset: '1px',
inset: '0.5px',
animationModifier: '.is-animated'
});

Expand Down
12 changes: 9 additions & 3 deletions packages/theming/demo/stories/ArrowStylesStory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,15 @@ const StyledDiv = styled.div<Omit<IArgs, 'isAnimated'>>`
box-shadow: ${p =>
p.hasBoxShadow &&
p.theme.shadows.lg(
'8px',
'12px',
getColor({ theme: p.theme, hue: 'chromeHue', shade: 600, transparency: 0.15 })
`${p.theme.space.base * (p.theme.colors.base === 'dark' ? 4 : 5)}px`,
`${p.theme.space.base * (p.theme.colors.base === 'dark' ? 5 : 6)}px`,
getColor({
theme: p.theme,
hue: 'neutralHue',
shade: 1200,
dark: { transparency: p.theme.opacity[800] },
light: { transparency: p.theme.opacity[200] }
})
)};
background-color: ${p => getColor({ theme: p.theme, variable: 'background.primary' })};
padding: ${p => p.theme.space.xxl};
Expand Down
10 changes: 5 additions & 5 deletions packages/theming/src/utils/arrowStyles.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import React from 'react';
import { render } from 'garden-test-utils';
import styled, { ThemeProps, DefaultTheme } from 'styled-components';
import { math } from 'polished';
import arrowStyles, { exponentialSymbols } from './arrowStyles';
import { math, stripUnit } from 'polished';
import arrowStyles from './arrowStyles';
import { ArrowPosition } from '../types';

interface IStyledDivProps extends ThemeProps<DefaultTheme> {
Expand All @@ -29,11 +29,11 @@ const StyledDiv = styled.div<IStyledDivProps>`
`;

const getArrowSize = (size = '6px') => {
return math(`${size} * 2 / sqrt(2)`, exponentialSymbols);
return `${Math.floor(((stripUnit(size) as number) * 2) / Math.sqrt(2)) + 1}px`;
};

const getArrowInset = (inset: string, size?: string) => {
return math(`${getArrowSize(size)} / -2 + ${inset}`);
return math(`${getArrowSize(size)} / -2 + ${inset} - 1`);
};

describe('arrowStyles', () => {
Expand All @@ -55,7 +55,7 @@ describe('arrowStyles', () => {

POSITION.forEach(position => {
const { container } = render(<StyledDiv arrowPosition={position} />);
const value = math(`${getArrowSize()} / -2`);
const value = math(`${getArrowSize()} / -2 - 1`);

expect(container.firstChild).toHaveStyleRule(position, value, { modifier: '::before' });
});
Expand Down
86 changes: 38 additions & 48 deletions packages/theming/src/utils/arrowStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { css, keyframes } from 'styled-components';
import { math } from 'polished';
import { math, stripUnit } from 'polished';
import { ArrowPosition } from '../types';

type ArrowOptions = {
Expand All @@ -15,24 +15,6 @@ type ArrowOptions = {
animationModifier?: string;
};

// Workaround for https://github.com/styled-components/polished/issues/550
export const exponentialSymbols = {
symbols: {
sqrt: {
func: {
symbol: 'sqrt',
f: (a: number) => Math.sqrt(a),
notation: 'func',
precedence: 0,
rightToLeft: 0,
argCount: 1
},
symbol: 'sqrt',
regSymbol: 'sqrt\\b'
}
}
};

const animationStyles = (position: ArrowPosition, modifier: string) => {
const property = position.split('-')[0];
/**
Expand All @@ -54,65 +36,77 @@ const animationStyles = (position: ArrowPosition, modifier: string) => {

const positionStyles = (position: ArrowPosition, size: string, inset: string) => {
const margin = math(`${size} / -2`);
const placement = math(`${margin} + ${inset}`);
const placement = math(`${margin} + ${inset} - 1`);
const offset = math(`${size} + 4`);
let clipPath;
let borderCss;
let positionCss;
let propertyRadius: string;

if (position.startsWith('top')) {
propertyRadius = 'border-bottom-right-radius';
clipPath = 'polygon(100% 0, 100% 1px, 1px 100%, 0 100%, 0 0)';
borderCss = css`
border-right: none;
border-bottom: none;
`;
positionCss = css`
top: ${placement};
right: ${position === 'top-right' && size};
left: ${position === 'top' ? '50%' : position === 'top-left' && size};
right: ${position === 'top-right' && offset};
left: ${position === 'top' ? '50%' : position === 'top-left' && offset};
margin-left: ${position === 'top' && margin};
`;
} else if (position.startsWith('right')) {
propertyRadius = 'border-bottom-left-radius';
clipPath = 'polygon(100% 0, 100% 100%, calc(100% - 1px) 100%, 0 1px, 0 0)';
borderCss = css`
border-bottom: none;
border-left: none;
`;
positionCss = css`
top: ${position === 'right' ? '50%' : position === 'right-top' && size};
top: ${position === 'right' ? '50%' : position === 'right-top' && offset};
right: ${placement};
bottom: ${position === 'right-bottom' && size};
bottom: ${position === 'right-bottom' && offset};
margin-top: ${position === 'right' && margin};
`;
} else if (position.startsWith('bottom')) {
propertyRadius = 'border-top-left-radius';
clipPath = 'polygon(100% 0, calc(100% - 1px) 0, 0 calc(100% - 1px), 0 100%, 100% 100%)';
borderCss = css`
border-top: none;
border-left: none;
`;
positionCss = css`
right: ${position === 'bottom-right' && size};
right: ${position === 'bottom-right' && offset};
bottom: ${placement};
left: ${position === 'bottom' ? '50%' : position === 'bottom-left' && size};
left: ${position === 'bottom' ? '50%' : position === 'bottom-left' && offset};
margin-left: ${position === 'bottom' && margin};
`;
} else if (position.startsWith('left')) {
propertyRadius = 'border-top-right-radius';
clipPath = 'polygon(0 100%, 100% 100%, 100% calc(100% - 1px), 1px 0, 0 0)';
borderCss = css`
border-top: none;
border-right: none;
`;
positionCss = css`
top: ${position === 'left' ? '50%' : position === 'left-top' && size};
bottom: ${size};
top: ${position === 'left' ? '50%' : position === 'left-top' && offset};
bottom: ${offset};
left: ${placement};
margin-top: ${position === 'left' && margin};
`;
}

/**
* 1. Round-off portion of the foreground square opposite the arrow tip
* (improved layout for IE which doesn't support 'clip-path').
* 2. Clip portion of the foreground square opposite the arrow tip so that it
* 1. Clip portion of the foreground square opposite the arrow tip so that it
* doesn't interfere with container content.
* 2. Knock out border opposite the arrow tip.
* 3. Arrow positioning on the base element.
*/
return css`
&::before {
${propertyRadius!}: 100%; /* [1] */
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

clip-path: ${clipPath}; /* [2] */
clip-path: ${clipPath}; /* [1] */
${borderCss}; /* [2] */
}

&::before,
&::after {
Copy link
Contributor

Choose a reason for hiding this comment

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

We might not even need an ::after after all! 😉 🙌

Screenshot 2024-05-23 at 7 01 53 AM

Copy link
Member Author

Choose a reason for hiding this comment

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

We need ::after to retain the box-shadow inheritance. Otherwise, the arrow pokes out un-shadowed. Not terrible, but the original designs wanted the shadow to follow the arrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Totally missed that but after some CSS tweaking I see it now. Thanks!

Screenshot 2024-05-23 at 10 27 29 AM

${positionCss}/* [3] */
${positionCss}; /* [3] */
}
`;
};
Expand Down Expand Up @@ -150,32 +144,28 @@ const positionStyles = (position: ArrowPosition, size: string, inset: string) =>
* @component
*/
export default function arrowStyles(position: ArrowPosition, options: ArrowOptions = {}) {
const size = options.size || '6px';
const size = options.size === undefined ? 6 : (stripUnit(options.size) as number);
const inset = options.inset || '0';
const squareSize = math(`${size} * 2 / sqrt(2)`, exponentialSymbols);
const squareSize = `${Math.floor((size * 2) / Math.sqrt(2)) + 1}px`;

/**
* 1. Set base positioning for an element with an arrow.
* 2. Allow any border inherited by `::after` to show through.
* 3. Border styling and box-shadow will be automatically inherited from the
* parent element.
* 2. Border styling will be inherited from the parent element.
* 3. Box shadow styling will be inherited from the parent element.
* 4. Apply shared sizing properties to ::before and ::after.
*/
return css`
position: relative; /* [1] */

&::before {
/* [2] */
border-width: inherit;
border-style: inherit;
border-color: transparent;
border: inherit;
background-clip: content-box;
}

&::after {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. ::after might be superfluous.

/* [3] */
z-index: -1;
border: inherit;
box-shadow: inherit;
}

Expand Down
21 changes: 9 additions & 12 deletions packages/tooltips/src/styled/StyledTooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,16 @@ const sizeStyles = ({
let arrowInset;

if (hasArrow) {
arrowInset = type === 'dark' ? '1px' : '0.5px';

if (size === 'small' || size === 'medium') {
arrowSize = margin;
arrowInset = type === 'dark' ? '1px' : '0';
} else {
arrowInset = type === 'dark' ? '2px' : '1px';

if (size === 'large') {
margin = `${theme.space.base * 2}px`;
arrowSize = margin;
} else if (size === 'extra-large') {
margin = `${theme.space.base * 3}px`;
arrowSize = `${theme.space.base * 2.5}px`;
}
arrowSize = `${theme.space.base * 1.25}px`;
} else if (size === 'large') {
margin = `${theme.space.base * 2}px`;
arrowSize = `${theme.space.base * 1.5}px`;
} else if (size === 'extra-large') {
margin = `${theme.space.base * 3}px`;
arrowSize = `${theme.space.base * 2}px`;
}
}

Expand Down