Skip to content

feat(drag-drop): adds light/dark mode colors #1805

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
merged 10 commits into from
May 16, 2024
Merged

feat(drag-drop): adds light/dark mode colors #1805

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bfb4f58
feat(drag-drop): updates package to v9 colors
geotrev May 7, 2024
47c2de2
chore: recolors DraggableList.DropIndicator
geotrev May 8, 2024
aa48dc5
chore: recolors dropzone
geotrev May 9, 2024
66033e3
fix: use 1000 opacity level for shadow in dark mode
geotrev May 9, 2024
17b5656
fix: use danger color for highlighted state
geotrev May 9, 2024
bf4b07b
refactor: clean up variable strings
geotrev May 9, 2024
18df789
chore: adds disabled variables
geotrev May 13, 2024
6cd2ccb
fix: use theme.shadows for draggable shadow
geotrev May 14, 2024
49c25e5
fix: uses verbose variable references for better searchability + code…
geotrev May 14, 2024
2fda125
chore: code review
geotrev May 16, 2024
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
98 changes: 98 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions packages/drag-drop/demo/~patterns/stories/components.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { Draggable, DraggableList, Dropzone } from '@zendeskgarden/react-drag-dr

import { animateLayoutChanges } from './utils';
import type { IDraggableItemProps, IDropIndicatorProps, ISortableColumnProps } from './types';
import styled from 'styled-components';
import { getColor } from '@zendeskgarden/react-theming';

export const DraggableItem = forwardRef<HTMLDivElement, IDraggableItemProps>((props, ref) => {
const { isOverlay, data, tabIndex, ...restProps } = props;
Expand Down Expand Up @@ -216,6 +218,10 @@ export const DraggableListItem = ({
);
};

const StyledTitle = styled.strong`
color: ${p => getColor({ variable: 'foreground.default', theme: p.theme })};
Copy link
Member

Choose a reason for hiding this comment

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

[nit] Garden never uses bold; should probably be styled with font-weight: ${p => p.theme.fontWeights.semibold};. Also, take care re: semantics of <strong> (add importance to) vs <b> (bring attention to).

Copy link
Contributor Author

@geotrev geotrev May 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I also just realized I can convert the former p to a more semantically correct h2, and leave the JSX alone.

Copy link
Member

Choose a reason for hiding this comment

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

[nit] the h2 still has the bold font weight problem. Would it be better to use Garden's <LG tag="h2"> typography component? We just don't want to confuse any consumers that view the pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that makes sense. I'll update.

`;

export const DraggablesColumn = ({
items,
hasPlaceholder,
Expand All @@ -226,7 +232,7 @@ export const DraggablesColumn = ({
return (
<div style={isHorizontal ? { minHeight: '100px' } : { width: '250px' }}>
<p>
<strong>Produce</strong>
<StyledTitle>Produce</StyledTitle>
</p>
{items.length > 0 && (
<DraggableList isHorizontal={isHorizontal}>
Expand All @@ -253,7 +259,7 @@ export const DroppablesColumn = (props: ISortableColumnProps) => {
return (
<div style={isHorizontal ? { minHeight: '100px' } : { width: '284px' }}>
<p>
<strong>Favorites</strong>
<StyledTitle>Favorites</StyledTitle>
</p>
<SortablesColumn {...props} />
</div>
Expand Down
90 changes: 1 addition & 89 deletions packages/drag-drop/src/elements/dropzone/Dropzone.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
*/

import React from 'react';
import { rgba } from 'polished';
import { render, renderRtl } from 'garden-test-utils';
import { Dropzone } from './Dropzone';
import { DEFAULT_THEME, getColorV8 } from '@zendeskgarden/react-theming';
import { DEFAULT_THEME } from '@zendeskgarden/react-theming';

describe('Dropzone', () => {
it('passes ref to underlying DOM element', () => {
Expand Down Expand Up @@ -154,91 +153,4 @@ describe('Dropzone', () => {
expect(queryByText('message')!.nodeName).toBe('P');
});
});

const STATES = ['default', 'danger', 'disabled'];
const dangerColor = getColorV8('dangerHue', 600, DEFAULT_THEME);
const dangerDarkColor = getColorV8('dangerHue', 800, DEFAULT_THEME);
const dangerBgColor = rgba(getColorV8('dangerHue', 600, DEFAULT_THEME) as string, 0.08);
const primaryColor = getColorV8('primaryHue', 600, DEFAULT_THEME);
const primaryDarkColor = getColorV8('primaryHue', 800, DEFAULT_THEME);
const primaryBgColor = rgba(getColorV8('primaryHue', 600, DEFAULT_THEME) as string, 0.08);
const neutralColor = getColorV8('neutralHue', 600, DEFAULT_THEME);

const StateMap: Record<string, any> = {
disabled: {
base: `
background-color: ${getColorV8('neutralHue', 200, DEFAULT_THEME)};
color: ${getColorV8('neutralHue', 400, DEFAULT_THEME)};
border-color: ${getColorV8('neutralHue', 300, DEFAULT_THEME)};
`
},
default: {
base: `
background-color: transparent;
color: ${neutralColor};
border-color: ${neutralColor};
`,
active: `
background-color: ${primaryBgColor};
color: ${primaryColor};
border-color: ${primaryColor};
`,
highlight: `
background-color: ${primaryBgColor};
color: ${primaryDarkColor};
border-color: ${primaryColor};
border-width: 2px;
border-style: solid;
`
},
danger: {
base: `
background-color: transparent;
color: ${dangerColor};
border-color: ${dangerColor};
`,
active: `
background-color: ${dangerBgColor};
color: ${dangerColor};
border-color: ${dangerColor};
`,
highlight: `
background-color: ${dangerBgColor};
color: ${dangerDarkColor};
border-color: ${dangerColor};
border-width: 2px;
border-style: solid;
`
}
};

describe.each(STATES)(`%s state`, state => {
it('applies correct base styling', () => {
const { container } = render(
<Dropzone isDanger={state === 'danger'} isDisabled={state === 'disabled'} />
);

expect(container.firstChild).toHaveStyle(StateMap[state].base);
});

if (StateMap[state].active) {
it('applies correct active styling', () => {
const { container } = render(
<Dropzone isDanger={state === 'danger'} isDisabled={state === 'disabled'} isActive />
);

expect(container.firstChild).toHaveStyle(StateMap[state].active);
});
}

if (StateMap[state].highlight) {
it('applies correct highlight styling', () => {
const { container } = render(
<Dropzone isDanger={state === 'danger'} isDisabled={state === 'disabled'} isHighlighted />
);

expect(container.firstChild).toHaveStyle(StateMap[state].highlight);
});
}
});
});
9 changes: 4 additions & 5 deletions packages/drag-drop/src/styled/draggable-list/StyledDropIndicator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import styled, { ThemeProps, DefaultTheme, css } from 'styled-components';
import { retrieveComponentStyles, DEFAULT_THEME, getColorV8 } from '@zendeskgarden/react-theming';
import { retrieveComponentStyles, DEFAULT_THEME, getColor } from '@zendeskgarden/react-theming';

const COMPONENT_ID = 'draggable_list.drop_indicator';

Expand All @@ -16,15 +16,14 @@ export interface IStyledDropIndicatorProps extends ThemeProps<DefaultTheme> {

const colorStyles = (props: IStyledDropIndicatorProps) => {
const { theme } = props;
const backgroundColor = getColorV8('primaryHue', 600, theme);
const color = getColorV8('primaryHue', 600, theme);
const color = getColor({ variable: 'foreground.primary', theme });
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const color = getColor({ variable: 'foreground.primary', theme });
const color = getColor({ variable: 'border.primaryEmphasis', theme });

I'm not seeing this in the designs, but semantically it feels a lot more like a border; conversely, I wouldn't expect this to change if the foreground was modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's fair. I was teetering between fg and border, anyway. :)


return css`
box-shadow: ${`0 0 0 ${theme.borderWidths.sm} ${color}`};
box-shadow: 0 0 0 ${theme.borderWidths.sm} ${color};

&::before,
&::after {
background-color: ${backgroundColor};
background-color: ${color};
}

&:focus {
Expand Down
Loading