Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
6b73276
onClick added to BarProps
Aug 23, 2024
e93ca6a
Added OnClick to Bar story
Aug 23, 2024
7cf5142
Made bar elements interactive
Aug 27, 2024
fd7c42a
Added onClick prop to bar element
Aug 27, 2024
e167cbe
Wrote mark click handler for chart render
Aug 27, 2024
093aa9d
defined mark click hook
Aug 27, 2024
4fc085b
wrote getAllMarkElements util function
Aug 27, 2024
ef72c15
markOnClick filter update
Aug 27, 2024
17388d3
cursor to pointer on enter for bar utils
Aug 27, 2024
ec95006
Added interactive logic for bar mark items, onClick default to undefi…
Aug 27, 2024
eff1c86
added additional bar onClick stories and getInteractive jest tests
Aug 28, 2024
ece73fd
lint fix
Aug 28, 2024
672be34
added click event testing
Aug 28, 2024
a899172
refactored getInteractive and getCursor for onClick prop check
Aug 28, 2024
e48d6a0
refactored getInteractive tests
Aug 28, 2024
d41dd8b
used barUpdateEncodings to control cursor
Aug 29, 2024
f15c159
removed extra interactive fields
Aug 30, 2024
66b0dc7
added cursor definitions for tests
Sep 3, 2024
a0de997
set interactive to false for background bars
Sep 3, 2024
3a125ec
Updated onMarkClick to register clicks as Actions
Sep 3, 2024
53aeb86
lint fix
Sep 3, 2024
994f98e
getCursor and getInteractive conditional fix
Sep 3, 2024
9964dfb
Merge pull request #1 from adrianTJenkins/bar-onClick
adrianTJenkins Sep 3, 2024
0798c2e
set background bar interactive to false, added props arg for rect
Sep 4, 2024
03677a3
restored interactive: false lines on barTestUtils
Sep 4, 2024
2845145
defaultProps for dodged and stacked bar stories
Sep 4, 2024
54ddf53
removed background cursor definition from getStackedBackgroundBar
Sep 4, 2024
bb30e28
removed cursor definitions in test cases
Sep 4, 2024
44e7961
Update src/utils/utils.ts
marshallpete Sep 4, 2024
4266556
Update src/utils/utils.ts
marshallpete Sep 4, 2024
7998b8c
Update src/utils/utils.ts
marshallpete Sep 4, 2024
454aea8
Update src/utils/utils.ts
marshallpete Sep 4, 2024
683d569
Merge branch 'adobe:main' into main
adrianTJenkins Sep 5, 2024
c070383
Merge branch 'main' of https://github.com/adrianTJenkins/react-spectr…
Sep 5, 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
8 changes: 6 additions & 2 deletions src/RscChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
} from '@constants';
import useChartImperativeHandle from '@hooks/useChartImperativeHandle';
import useLegend from '@hooks/useLegend';
import useMarkOnClicks from '@hooks/useMarkOnClicks';
import usePopoverAnchorStyle from '@hooks/usePopoverAnchorStyle';
import usePopovers, { PopoverDetail } from '@hooks/usePopovers';
import useSpec from '@hooks/useSpec';
Expand Down Expand Up @@ -152,6 +153,7 @@ export const RscChart = forwardRef<ChartHandle, RscChartProps>(

const tooltips = useTooltips(sanitizedChildren);
const popovers = usePopovers(sanitizedChildren);
const onMarkClicks = useMarkOnClicks(sanitizedChildren);

// gets the correct css style to display the anchor in the correct position
const targetStyle = usePopoverAnchorStyle(
Expand Down Expand Up @@ -259,7 +261,7 @@ export const RscChart = forwardRef<ChartHandle, RscChartProps>(
tooltipHandler.call(this, event, item, value);
}
});
if (popovers.length || legendIsToggleable || onLegendClick) {
if (popovers.length || onMarkClicks.length || legendIsToggleable || onLegendClick) {
if (legendIsToggleable) {
view.signal('hiddenSeries', legendHiddenSeries);
}
Expand All @@ -278,7 +280,9 @@ export const RscChart = forwardRef<ChartHandle, RscChartProps>(
selectedDataName,
setLegendHiddenSeries,
legendIsToggleable,
onLegendClick
onLegendClick,
onMarkClicks,

)
);
}
Expand Down
42 changes: 42 additions & 0 deletions src/hooks/useMarkOnClicks.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2023 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
import { createElement, useMemo } from 'react';

import { getAllMarkElements } from '@utils';

import { Chart } from '../Chart';
import { Bar } from '../components/Bar';
import { BarElement, ChartChildElement, Datum } from '../types';

type MappedMarkElement = { name: string; element: BarElement };

export type MarkDetail = {
markName?: string;
onClick?: (datum: Datum) => void;
};

export default function useMarkOnClicks(children: ChartChildElement[]): MarkDetail[] {
const markElements = useMemo(
() => getAllMarkElements(createElement(Chart, { data: [] }, children), Bar, []) as MappedMarkElement[],
[children]
);
return useMemo(
() =>
markElements
.filter((mark) => mark.element.props.onClick)
.map((mark) => ({
markName: mark.name,
onClick: mark.element.props.onClick,
})) as MarkDetail[],
[markElements]
);
}
3 changes: 0 additions & 3 deletions src/specBuilder/bar/barTestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ export const stackedLabelWithStyles = {
from: { data: FILTERED_TABLE },
name: 'bar0_annotationBackground',
interactive: false,

encode: {
enter: {
align: { value: 'center' },
Expand Down Expand Up @@ -145,7 +144,6 @@ export const stackedLabelBackground = {
from: { data: FILTERED_TABLE },
name: 'bar0_annotationBackground',
interactive: false,

encode: {
enter: {
align: { value: 'center' },
Expand Down Expand Up @@ -173,7 +171,6 @@ export const stackedLabelText = {
from: { data: FILTERED_TABLE },
name: 'bar0_annotationText',
interactive: false,
Copy link
Member

Choose a reason for hiding this comment

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

Do you know what code got added/modified that caused these to be removed?

Copy link
Contributor Author

@adrianTJenkins adrianTJenkins Sep 4, 2024

Choose a reason for hiding this comment

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

I'm not sure on this. I don't remember modifying myself. I will restore the interactive: false lines


encode: {
enter: {
x: { scale: stackedXScale, field: defaultBarProps.dimension, band: 0.5 },
Expand Down
2 changes: 1 addition & 1 deletion src/specBuilder/bar/barUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export const getBarEnterEncodings = ({ children, color, colorScheme, name, opaci
});

export const getBarUpdateEncodings = (props: BarSpecProps): EncodeEntry => ({
cursor: getCursor(props.children),
cursor: getCursor(props.children, props),
opacity: getMarkOpacity(props),
stroke: getStroke(props),
strokeDash: getStrokeDash(props),
Expand Down
2 changes: 1 addition & 1 deletion src/specBuilder/bar/dodgedBarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const getDodgedMark = (props: BarSpecProps): GroupMark => {
name,
from: { data: `${name}_facet` },
type: 'rect',
interactive: getInteractive(children),
interactive: getInteractive(children, props),
encode: {
enter: {
...getBaseBarEnterEncodings(props),
Expand Down
2 changes: 1 addition & 1 deletion src/specBuilder/bar/stackedBarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const getStackedBar = (props: BarSpecProps): RectMark => {
name,
type: 'rect',
from: { data: isDodgedAndStacked(props) ? `${name}_facet` : getBaseDataSourceName(props) },
interactive: getInteractive(children),
interactive: getInteractive(children, props),
encode: {
enter: {
...getBaseBarEnterEncodings(props),
Expand Down
12 changes: 12 additions & 0 deletions src/specBuilder/marks/markUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
getSymbolSizeProductionRule,
getTooltip,
getXProductionRule,
getInteractive,
hasMetricRange,
hasTooltip,
} from './markUtils';
Expand Down Expand Up @@ -224,6 +225,17 @@ describe('getXProductionRule()', () => {
});
});

describe('getInteractive()', () => {
const tooltip = createElement(ChartTooltip);
const popover = createElement(ChartPopover);
test('should return true based on having interactive children', () => {
expect(getInteractive([tooltip])).toEqual(true);
expect(getInteractive([])).toEqual(false);
expect(getInteractive([tooltip, popover])).toEqual(true);
expect(getInteractive(defaultBarProps.children, defaultBarProps)).toEqual(false);
});
});

describe('getColorProductionRuleSignalString()', () => {
test('should return signal reference if color is an array', () => {
expect(
Expand Down
12 changes: 6 additions & 6 deletions src/specBuilder/marks/markUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,21 +71,21 @@ import {
} from '../../types';

/**
* If a popover exists on the mark, then set the cursor to a pointer.
* If a popover or onClick prop exists on the mark, then set the cursor to a pointer.
*/
export function getCursor(children: MarkChildElement[]): ScaledValueRef<Cursor> | undefined {
if (hasPopover(children)) {
export function getCursor(children: MarkChildElement[], props?: BarSpecProps): ScaledValueRef<Cursor> | undefined {
if ((props?.onClick !== undefined) || hasPopover(children)) {
return { value: 'pointer' };
}
}

/**
* If there aren't any tooltips or popovers on the mark, then set interactive to false.
* If there aren't any tooltips, popovers, or onClick props on the mark, then set interactive to false.
* This prevents the mark from interfering with other interactive marks.
*/
export function getInteractive(children: MarkChildElement[]): boolean {
export function getInteractive(children: MarkChildElement[], props?: BarSpecProps): boolean {
// skip annotations
return hasInteractiveChildren(children);
return (props?.onClick !== undefined) || hasInteractiveChildren(children);
}

/**
Expand Down
37 changes: 21 additions & 16 deletions src/stories/components/Bar/Bar.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import React, { ReactElement, createElement } from 'react';

import { Annotation } from '@components/Annotation';
import useChartProps from '@hooks/useChartProps';
import { Axis, Bar, Chart } from '@rsc';
import { Axis, Bar, BarProps, Chart } from '@rsc';
import { StoryFn } from '@storybook/react';
import { bindWithProps } from '@test-utils';

Expand All @@ -35,54 +35,59 @@ const BarStory: StoryFn<typeof Bar> = (args): ReactElement => {
);
};

const Basic = bindWithProps(BarStory);
Basic.args = {
const defaultProps: BarProps = {
dimension: 'browser',
metric: 'downloads',
onClick: undefined,
}

const Basic = bindWithProps(BarStory);
Basic.args = {
...defaultProps
};

const Horizontal = bindWithProps(BarStory);
Horizontal.args = {
dimension: 'browser',
metric: 'downloads',
...defaultProps,
orientation: 'horizontal',
};

const LineType = bindWithProps(BarStory);
LineType.args = {
dimension: 'browser',
metric: 'downloads',
...defaultProps,
opacity: { value: 0.75 },
lineType: { value: 'dashed' },
lineWidth: 2,
};

const Opacity = bindWithProps(BarStory);
Opacity.args = {
dimension: 'browser',
metric: 'downloads',
...defaultProps,
opacity: { value: 0.75 },
};

const PaddingRatio = bindWithProps(BarStory);
PaddingRatio.args = {
dimension: 'browser',
metric: 'downloads',
...defaultProps,
paddingRatio: 0.2,
};

const WithAnnotation = bindWithProps(BarStory);
WithAnnotation.args = {
dimension: 'browser',
...defaultProps,
children: createElement(Annotation, { textKey: 'percentLabel' }),
metric: 'downloads',
};

const HasSquareCorners = bindWithProps(BarStory);
HasSquareCorners.args = {
dimension: 'browser',
metric: 'downloads',
...defaultProps,
hasSquareCorners: true,
};

export { Basic, Horizontal, LineType, Opacity, PaddingRatio, WithAnnotation, HasSquareCorners };
const OnClick = bindWithProps(BarStory);
OnClick.args = {
dimension: 'browser',
metric: 'downloads',
}

export { Basic, Horizontal, LineType, Opacity, PaddingRatio, WithAnnotation, HasSquareCorners, OnClick };
29 changes: 25 additions & 4 deletions src/stories/components/Bar/Bar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
import React from 'react';

import '@matchMediaMock';
import { Bar } from '@rsc';
import { findAllMarksByGroupName, findChart, render } from '@test-utils';
import { clickNthElement, findAllMarksByGroupName, findChart, render } from '@test-utils';

import { Basic, Opacity, PaddingRatio, WithAnnotation } from './Bar.story';
import { Basic, Opacity, PaddingRatio, WithAnnotation, OnClick } from './Bar.story';
import { Color, DodgedStacked } from './DodgedBar.story';
import { Basic as StackedBasic } from './StackedBar.story';
import { barData } from './data';

describe('Bar', () => {
// Bar is not a real React component. This is test just provides test coverage for sonarqube
Expand Down Expand Up @@ -98,4 +97,26 @@ describe('Bar', () => {
const bars = await findAllMarksByGroupName(chart, 'bar0');
expect(bars.length).toEqual(9);
});

test('should call onClick callback when selecting a bar item', async () => {
const onClick = jest.fn();
render(<OnClick {...OnClick.args} onClick={onClick} />);
const chart = await findChart();
const bars = await findAllMarksByGroupName(chart, 'bar0');

await clickNthElement(bars, 0);
expect(onClick).toHaveBeenCalledWith(expect.objectContaining(barData[0]));

await clickNthElement(bars, 1);
expect(onClick).toHaveBeenCalledWith(expect.objectContaining(barData[1]));

await clickNthElement(bars, 2);
expect(onClick).toHaveBeenCalledWith(expect.objectContaining(barData[2]));

await clickNthElement(bars, 3);
expect(onClick).toHaveBeenCalledWith(expect.objectContaining(barData[3]));

await clickNthElement(bars, 4);
expect(onClick).toHaveBeenCalledWith(expect.objectContaining(barData[4]));
});
});
36 changes: 22 additions & 14 deletions src/stories/components/Bar/DodgedBar.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import React, { ReactElement, createElement } from 'react';

import { Annotation } from '@components/Annotation';
import useChartProps from '@hooks/useChartProps';
import { Axis, Bar, Chart, ChartPopover, ChartTooltip, Legend, categorical6 } from '@rsc';
import { Axis, Bar, BarProps, Chart, ChartPopover, ChartTooltip, Legend, categorical6 } from '@rsc';
import { StoryFn } from '@storybook/react';
import { bindWithProps } from '@test-utils';

Expand Down Expand Up @@ -82,25 +82,28 @@ const DodgedBarLineTypeStory: StoryFn<typeof Bar> = (args): ReactElement => {
);
};

const Color = bindWithProps(DodgedBarStory);
Color.args = {
const defaultProps: BarProps = {
type: 'dodged',
dimension: 'browser',
onClick: undefined,
}

const Color = bindWithProps(DodgedBarStory);
Color.args = {
...defaultProps,
order: 'order',
color: 'operatingSystem',
};

const DodgedStacked = bindWithProps(DodgedBarStory);
DodgedStacked.args = {
type: 'dodged',
dimension: 'browser',
...defaultProps,
color: ['operatingSystem', 'version'],
};

const LineType = bindWithProps(DodgedBarLineTypeStory);
LineType.args = {
type: 'dodged',
dimension: 'browser',
...defaultProps,
order: 'order',
lineType: 'operatingSystem',
lineWidth: 2,
Expand All @@ -109,27 +112,32 @@ LineType.args = {

const Opacity = bindWithProps(DodgedBarStory);
Opacity.args = {
type: 'dodged',
dimension: 'browser',
...defaultProps,
order: 'order',
opacity: 'operatingSystem',
};

const Popover = bindWithProps(DodgedBarPopoverStory);
Popover.args = {
type: 'dodged',
dimension: 'browser',
...defaultProps,
order: 'order',
color: 'operatingSystem',
};

const DodgedStackedWithLabels = bindWithProps(DodgedBarStory);
DodgedStackedWithLabels.args = {
type: 'dodged',
dimension: 'browser',
...defaultProps,
color: ['operatingSystem', 'version'],
children: createElement(Annotation, { textKey: 'percentLabel' }),
paddingRatio: 0.1,
};

export { Color, DodgedStacked, DodgedStackedWithLabels, LineType, Opacity, Popover };
const OnClick = bindWithProps(DodgedBarStory);
OnClick.args = {
type: 'dodged',
dimension: 'browser',
order: 'order',
color: 'operatingSystem',
};

export { Color, DodgedStacked, DodgedStackedWithLabels, LineType, Opacity, Popover, OnClick };
Loading