Skip to content

refactor(render-methods): deprecate renderXYZ methods #436

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

Merged
merged 5 commits into from
Apr 17, 2020
Merged
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 docs/Guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ Global Styles compliant to the Fiori 2.0 Design Guidelines are located in [ui5-w
- All Event handlers **must** start with `on`.<br />
e.g. `onClick`, `onSelect`, `onSelectionChange`, .etc<br />
All Events must pass a instance of the `Event`-Class as single parameter.
- When passing additional elements into a component, a render function should be used. This prop must start with `render`<br />
e.g. `renderExtension`, `renderHeaderContent`, .etc
- When passing additional elements into a component, a slot should be used. This prop should contain a `ReactNode` or an array of ReactNodes (`ReactNode[]` or `ReactNodeArray`)
- If you have a stateful component and use `getDerivedStateFromProps`, please store the previous props in a `prevProps` object in the state.<br />
e.g. `state = { selectedKey: 1, prevProps: { selectedKey: 2 } }`

Expand Down
17 changes: 17 additions & 0 deletions packages/base/src/hooks/useDeprecateRenderMethods.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { useEffect, useMemo } from 'react';
import { deprecationNotice } from '../utils';

export const useDeprecateRenderMethods = (props, renderMethodName, slotName) => {
useEffect(() => {
if (props[renderMethodName]) {
deprecationNotice(
`${renderMethodName}`,
`The prop '${renderMethodName}' is deprecated and will be removed in the next major release.\nPlease use '${slotName}' instead.`
);
}
}, []);

return useMemo(() => {
return props[slotName] ?? (typeof props[renderMethodName] === 'function' ? props[renderMethodName]() : null);
}, [props[renderMethodName], props[slotName]]);
};
Empty file.
6 changes: 6 additions & 0 deletions packages/base/src/lib/hooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { useConsolidatedRef } from '../hooks/useConsolidatedRef';
import { useDeprecateRenderMethods } from '../hooks/useDeprecateRenderMethods';
import { usePassThroughHtmlProps } from '../hooks/usePassThroughHtmlProps';
import { useViewportRange } from '../hooks/useViewportRange';

export { useConsolidatedRef, useDeprecateRenderMethods, usePassThroughHtmlProps, useViewportRange };
20 changes: 5 additions & 15 deletions packages/base/src/styling/HSLColor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,33 +84,23 @@ export class HSLColor {
}

static lighten(color: HSLColor | string, amount: number) {
return HSLColor.of(color)
.clone()
.lighten(amount);
return HSLColor.of(color).clone().lighten(amount);
}

static darken(color: HSLColor | string, amount: number) {
return HSLColor.of(color)
.clone()
.darken(amount);
return HSLColor.of(color).clone().darken(amount);
}

static saturate(color: HSLColor | string, amount: number) {
return HSLColor.of(color)
.clone()
.saturate(amount);
return HSLColor.of(color).clone().saturate(amount);
}

static desaturate(color: HSLColor | string, amount: number) {
return HSLColor.of(color)
.clone()
.desaturate(amount);
return HSLColor.of(color).clone().desaturate(amount);
}

static hsla(color: HSLColor | string, amount: number) {
return HSLColor.of(color)
.clone()
.setAlpha(amount);
return HSLColor.of(color).clone().setAlpha(amount);
}

getHue() {
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/src/components/RadialChart/RadialChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface RadialChartProps extends CommonProps {
maxValue?: number;
displayValue?: number | string;
color?: CSSProperties['color'];
onDataPointClick?: (event: CustomEvent<{value: unknown; payload: unknown; xIndex: number}>) => void;
onDataPointClick?: (event: CustomEvent<{ value: unknown; payload: unknown; xIndex: number }>) => void;
height?: number | string;
width?: number | string;
}
Expand Down
19 changes: 8 additions & 11 deletions packages/charts/src/internal/withChartContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,15 @@ export const withChartContainer = (Component: ComponentType<any>) => {

const chartHeight = useMemo(() => (noLegend ? height : height - 60), [noLegend, height]);

const chartWrapperStyles: CSSProperties = useMemo(
() => {
let innerChartWrapperStyles : CSSProperties = {
position: 'relative',
height: chartHeight >= 0 ? `${chartHeight}px` : 'auto',
width: width ? `${width}px` : 'auto'
};
const chartWrapperStyles: CSSProperties = useMemo(() => {
let innerChartWrapperStyles: CSSProperties = {
position: 'relative',
height: chartHeight >= 0 ? `${chartHeight}px` : 'auto',
width: width ? `${width}px` : 'auto'
};

return innerChartWrapperStyles
},
[chartHeight, width]
);
return innerChartWrapperStyles;
}, [chartHeight, width]);

return (
<div ref={outerContainer} className={classNames} style={inlineStyle} title={tooltip} slot={slot}>
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/src/util/Utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const useMergedConfig = (x, y) => {

// this needs to be a function as we need the `this` of the chart;
export const formatAxisCallback = (formatter) =>
function(value) {
function (value) {
// @ts-ignore
const currentDataset = this.chart.data.datasets[0];
return formatter(value, currentDataset);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,14 @@ describe('AnalyticalTable', () => {
expect(wrapper.render()).toMatchSnapshot();

// test asc function inside the popover element
let component = wrapper
.find('ui5-li')
.at(1)
.instance();
let component = wrapper.find('ui5-li').at(1).instance();
// @ts-ignore
component.click();

expect(wrapper.render()).toMatchSnapshot();

// test desc function inside the popover element
component = wrapper
.find('ui5-li')
.at(0)
.instance();
component = wrapper.find('ui5-li').at(0).instance();
// @ts-ignore
component.click();

Expand All @@ -186,10 +180,7 @@ describe('AnalyticalTable', () => {
/>
);

const colInst = wrapper
.find('div[role="columnheader"]')
.at(0)
.instance();
const colInst = wrapper.find('div[role="columnheader"]').at(0).instance();

// @ts-ignore
expect(colInst.draggable).toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export const tableWithExtension = () => {
filterable={boolean('filterable', true)}
visibleRows={number('visibleRows', 15)}
groupable={boolean('groupable', true)}
renderExtension={() => <Button>Hello from the Table Extension!</Button>}
extension={<Button>Hello from the Table Extension!</Button>}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ValueState } from "@ui5/webcomponents-react/lib/ValueState";
import { ValueState } from '@ui5/webcomponents-react/lib/ValueState';

const getRandomArrayEntry = (array) => array[Math.floor(Math.random() * array.length)];

Expand Down Expand Up @@ -38,7 +38,9 @@ const newEntry = () => {
name: getRandomName(),
age: getRandomNumber(18, 65)
},
status: [ValueState.None, ValueState.Information, ValueState.Success, ValueState.Warning, ValueState.Error][Math.floor(Math.random() * 4)]
status: [ValueState.None, ValueState.Information, ValueState.Success, ValueState.Warning, ValueState.Error][
Math.floor(Math.random() * 4)
]
};
};

Expand All @@ -63,7 +65,9 @@ const makeEntry = () => ({
name: getRandomName(),
age: getRandomNumber(18, 65)
},
status: [ValueState.None, ValueState.Information, ValueState.Success, ValueState.Warning, ValueState.Error][Math.floor(Math.random() * 4)]
status: [ValueState.None, ValueState.Information, ValueState.Success, ValueState.Warning, ValueState.Error][
Math.floor(Math.random() * 4)
]
});

const generateData = (numEntries, isTree = false) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const columnsDeps = (deps, { instance: { state, webComponentsReactProperties } }
];

const columns = (columns, { instance }) => {

if (!instance.state || !instance.rows) {
return columns;
}
Expand Down
8 changes: 5 additions & 3 deletions packages/main/src/components/AnalyticalTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createComponentStyles } from '@ui5/webcomponents-react-base/lib/createC
import { StyleClassHelper } from '@ui5/webcomponents-react-base/lib/StyleClassHelper';
import { usePassThroughHtmlProps } from '@ui5/webcomponents-react-base/lib/usePassThroughHtmlProps';
import { enrichEventWithDetails } from '@ui5/webcomponents-react-base/lib/Utils';
import { useDeprecateRenderMethods } from '@ui5/webcomponents-react-base/lib/hooks';
import { TableScaleWidthMode } from '@ui5/webcomponents-react/lib/TableScaleWidthMode';
import { TableSelectionBehavior } from '@ui5/webcomponents-react/lib/TableSelectionBehavior';
import { TableSelectionMode } from '@ui5/webcomponents-react/lib/TableSelectionMode';
Expand Down Expand Up @@ -71,6 +72,7 @@ export interface TableProps extends CommonProps {
* Extension section of the Table. If not set, no extension area will be rendered
*/
renderExtension?: () => ReactNode;
extension?: ReactNode;

// appearance

Expand Down Expand Up @@ -137,7 +139,6 @@ const AnalyticalTable: FC<TableProps> = forwardRef((props: TableProps, ref: Ref<
style,
tooltip,
title,
renderExtension,
loading,
groupBy,
selectionMode,
Expand Down Expand Up @@ -176,6 +177,7 @@ const AnalyticalTable: FC<TableProps> = forwardRef((props: TableProps, ref: Ref<
const [analyticalTableRef, reactWindowRef] = useTableScrollHandles(ref);
const tableRef: RefObject<HTMLDivElement> = useRef();
const resizeObserverInitialized = useRef(false);
const extension = useDeprecateRenderMethods(props, 'renderExtension', 'extension');

const getSubRows = useCallback((row) => row[subRowsKey] || [], [subRowsKey]);

Expand Down Expand Up @@ -257,7 +259,7 @@ const AnalyticalTable: FC<TableProps> = forwardRef((props: TableProps, ref: Ref<
useEffect(() => {
// @ts-ignore
const tableWidthObserver = new ResizeObserver(() => {
if(resizeObserverInitialized.current) {
if (resizeObserverInitialized.current) {
updateTableClientWidth();
}
resizeObserverInitialized.current = true;
Expand Down Expand Up @@ -359,7 +361,7 @@ const AnalyticalTable: FC<TableProps> = forwardRef((props: TableProps, ref: Ref<
return (
<div className={className} style={inlineStyle} title={tooltip} ref={analyticalTableRef} {...passThroughProps}>
{title && <TitleBar>{title}</TitleBar>}
{typeof renderExtension === 'function' && <div>{renderExtension()}</div>}
{extension && <div>{extension}</div>}
<div className={tableContainerClasses.valueOf()} ref={tableRef}>
{
<div
Expand Down
12 changes: 4 additions & 8 deletions packages/main/src/components/Bar/Bar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ describe('Bar', () => {
test('Render all content', () => {
const wrapper = mount(
<Bar
renderContentLeft={() => <div>Content Left</div>}
renderContentMiddle={() => <div>Content Middle</div>}
renderContentRight={() => <div>Content Right</div>}
contentLeft={<div>Content Left</div>}
contentMiddle={<div>Content Middle</div>}
contentRight={<div>Content Right</div>}
/>
);
expect(wrapper.render()).toMatchSnapshot();
Expand All @@ -29,11 +29,7 @@ describe('Bar', () => {
const text3 = 'Content Right';

const wrapper = mount(
<Bar
renderContentLeft={() => <div>{text1}</div>}
renderContentMiddle={() => <div>{text2}</div>}
renderContentRight={() => <div>{text3}</div>}
/>
<Bar contentLeft={<div>{text1}</div>} contentMiddle={<div>{text2}</div>} contentRight={<div>{text3}</div>} />
);
expect(wrapper.text()).toContain(text1);
expect(wrapper.text()).toContain(text2);
Expand Down
6 changes: 3 additions & 3 deletions packages/main/src/components/Bar/demo.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ export const defaultStory = () => {
return (
<Bar
design={select('design', BarDesign, BarDesign.Auto)}
renderContentLeft={() => <Label>Content Left</Label>}
renderContentMiddle={() => <Label>Content Middle</Label>}
renderContentRight={() => <Label>Content Right</Label>}
contentLeft={<Label>Content Left</Label>}
contentMiddle={<Label>Content Middle</Label>}
contentRight={<Label>Content Right</Label>}
/>
);
};
Expand Down
22 changes: 13 additions & 9 deletions packages/main/src/components/Bar/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { StyleClassHelper } from '@ui5/webcomponents-react-base/lib/StyleClassHelper';
import { usePassThroughHtmlProps } from '@ui5/webcomponents-react-base/lib/usePassThroughHtmlProps';
import React, { FC, forwardRef, Ref } from 'react';
import { useDeprecateRenderMethods } from '@ui5/webcomponents-react-base/lib/hooks';
import React, { FC, forwardRef, ReactNode, Ref } from 'react';
import { createComponentStyles } from '@ui5/webcomponents-react-base/lib/createComponentStyles';
import { CommonProps } from '../../interfaces/CommonProps';
import { BarDesign } from '../../lib/BarDesign';
Expand All @@ -10,6 +11,9 @@ export interface BarPropTypes extends CommonProps {
renderContentLeft?: () => JSX.Element;
renderContentMiddle?: () => JSX.Element;
renderContentRight?: () => JSX.Element;
contentLeft: ReactNode | ReactNode[];
contentMiddle: ReactNode | ReactNode[];
contentRight: ReactNode | ReactNode[];
design?: BarDesign;
}

Expand All @@ -19,7 +23,10 @@ const useStyles = createComponentStyles(styles, { name: 'Bar' });
* <code>import { Bar } from '@ui5/webcomponents-react/lib/Bar';</code>
*/
const Bar: FC<BarPropTypes> = forwardRef((props: BarPropTypes, ref: Ref<HTMLDivElement>) => {
const { renderContentLeft, renderContentMiddle, renderContentRight, className, style, tooltip, slot, design } = props;
const { className, style, tooltip, slot, design } = props;
const contentLeft = useDeprecateRenderMethods(props, 'renderContentLeft', 'contentLeft');
const contentMiddle = useDeprecateRenderMethods(props, 'renderContentMiddle', 'contentMiddle');
const contentRight = useDeprecateRenderMethods(props, 'renderContentRight', 'contentRight');

const classes = useStyles();

Expand Down Expand Up @@ -56,24 +63,21 @@ const Bar: FC<BarPropTypes> = forwardRef((props: BarPropTypes, ref: Ref<HTMLDivE
{...passThroughProps}
>
<div data-bar-part="Left" className={classes.left}>
{renderContentLeft()}
{contentLeft}
</div>
<div data-bar-part="Center" className={classes.center}>
<div className={classes.inner}>{renderContentMiddle()}</div>
<div className={classes.inner}>{contentMiddle}</div>
</div>
<div data-bar-part="Right" className={classes.right}>
{renderContentRight()}
{contentRight}
</div>
</div>
);
});

Bar.displayName = 'Bar';
Bar.defaultProps = {
design: BarDesign.Auto,
renderContentLeft: () => null,
renderContentMiddle: () => null,
renderContentRight: () => null
design: BarDesign.Auto
};

export { Bar };
Loading