Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

feat: 'render' callback for shorthand props #519

Merged
merged 23 commits into from
Dec 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
bca8450
introduce basic support for 'render' callback
kuzhelov-ms Nov 23, 2018
9fe1514
support custom tree rendering
kuzhelov-ms Nov 23, 2018
fa9e34c
introduce shortcut syntax support
kuzhelov-ms Nov 23, 2018
54a247f
remove renderIcon from Button API
kuzhelov-ms Nov 23, 2018
c262702
small refactoring adjustments
kuzhelov-ms Nov 23, 2018
e5d08b8
remove duplicated logic
kuzhelov-ms Nov 23, 2018
f32a87e
adjust types
kuzhelov-ms Nov 23, 2018
f072a59
address review comment
kuzhelov-ms Nov 27, 2018
6529da2
refactor introduced factory method
kuzhelov-ms Nov 27, 2018
7fd942d
remove variadic overload
kuzhelov-ms Nov 29, 2018
11fbbff
remove 'renderX' props from components' API
kuzhelov-ms Dec 2, 2018
d63bf2c
add render callback tests
kuzhelov-ms Dec 3, 2018
c4f01bd
Merge branch 'master' into feat/shorthand-render-callback
kuzhelov Dec 3, 2018
27e32e1
merge master
kuzhelov-ms Dec 3, 2018
3cbef28
introduce 'render' as a default rendering option
kuzhelov-ms Dec 3, 2018
71eeab6
fix Avatar example
kuzhelov-ms Dec 3, 2018
2b4d886
remove code duplication
kuzhelov-ms Dec 3, 2018
971f26e
Merge branch 'master' into feat/shorthand-render-callback
kuzhelov Dec 3, 2018
e36be12
introduce custom tree rendering tests
kuzhelov-ms Dec 4, 2018
a3399e4
Merge branch 'feat/shorthand-render-callback' of https://github.com/s…
kuzhelov-ms Dec 4, 2018
67929d4
Merge branch 'master' into feat/shorthand-render-callback
kuzhelov Dec 4, 2018
8e0e614
update changelog
kuzhelov-ms Dec 4, 2018
f20320f
add render props for panel title and content of Accordion
kuzhelov-ms Dec 4, 2018
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: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Add `react-dom` as available import in the editor @mnajdova ([#553](https://github.com/stardust-ui/react/pull/553))
- Fix incorrect and missing filled or outline versions of Teams SVG icons @codepretty ([#552](https://github.com/stardust-ui/react/pull/552))

### Features
- Add `render` callback as an option for shorthand value @kuzhelov ([#519](https://github.com/stardust-ui/react/pull/519))

<!--------------------------------[ v0.13.1 ]------------------------------- -->
## [v0.13.1](https://github.com/stardust-ui/react/tree/v0.13.1) (2018-12-03)
[Compare changes](https://github.com/stardust-ui/react/compare/v0.13.0...v0.13.1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ const AvatarExampleImageCustomizationShorthand = () => (
/>
&emsp;
<Avatar
renderImage={(Image, props, children) => (
image={
<Icon name="user" circular variables={{ color: 'blue' }} styles={{ padding: '8px' }} />
)}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

status={{ color: 'green', icon: 'check', title: 'Available' }}
/>
</>
Expand Down
34 changes: 16 additions & 18 deletions src/components/Accordion/Accordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import AccordionTitle from './AccordionTitle'
import AccordionContent from './AccordionContent'
import { defaultBehavior } from '../../lib/accessibility'
import { Accessibility } from '../../lib/accessibility/types'

import {
ComponentEventHandler,
Extendable,
ShorthandRenderFunction,
ShorthandValue,
ShorthandRenderFunction,
} from '../../../types/utils'

export interface AccordionProps extends UIComponentProps, ChildrenComponentProps {
Expand Down Expand Up @@ -46,24 +47,20 @@ export interface AccordionProps extends UIComponentProps, ChildrenComponentProps
}[]

/**
* A custom render iterator for rendering each Accordion panel content.
* The default component, props, and children are available for each panel content.
* A custom renderer for each Accordion's panel title.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
* @param {React.ReactType} Component - The panel's component type.
* @param {object} props - The panel's computed props.
*/
renderContent?: ShorthandRenderFunction
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, I find this methods useful. They are not callback render for a shorthand, but for each item of array. Do we want to get rid of this as well? It would be nice from user perspective for the array shorthands, to be able to define this logic via an API property. What are your thoughts on this @kuzhelov ?

Copy link
Contributor Author

@kuzhelov kuzhelov Dec 3, 2018

Choose a reason for hiding this comment

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

Solution with no additional API changes

// this could be a helper util
const customTreeShorthand = (value, renderTree) => (render => render(value, renderTree))

const renderCustomMenuItem = (Component, props) => <Component {...props} />

<Menu items={[ 
    { as: ..., ... },
    ...
  ]
 .map(shorthandValue => customTreeShorthand(shorthandValue, renderCustomMenuItem))
} 
.. />

Solution with render method introduced for limited set of components - specifically, collection and hierarchical ones: Menu, Tree, Accordion, List (?)

Needs changes, but we may talk about the final result even now:

<Menu items={[ 
    { as: ..., ... },
    ...
  ]}
  renderItem={(Component, props) => ... }  { /* is treated a default rendering logic, if not overwritten by shorthand */ }
.. />

What I don't like here is that it is not absolutely clear for the client where Component and props have come from, and where the data that is provided as shorthand object (i.e. { as: ..., ...} resides in those Component and props).

Copy link
Contributor

Choose a reason for hiding this comment

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

I am okay with both approaches, but when we have shorthand array, with two shorthands inside, as the panels are inside the Accordion, the user will have to define two maps for both shorthand, which may look awkward... Let's hear other folks' opinion before deciding.

renderPanelTitle?: ShorthandRenderFunction

/**
* A custom render iterator for rendering each Accordion panel title.
* The default component, props, and children are available for each panel title.
* A custom renderer for each Accordion's panel content.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
* @param {React.ReactType} Component - The panel's component type.
* @param {object} props - The panel's computed props.
*/
renderTitle?: ShorthandRenderFunction
renderPanelContent?: ShorthandRenderFunction

/**
* Accessibility behavior if overridden by the user.
Expand Down Expand Up @@ -104,8 +101,9 @@ class Accordion extends AutoControlledComponent<Extendable<AccordionProps>, any>
),
]),
accessibility: PropTypes.func,
renderTitle: PropTypes.func,
renderContent: PropTypes.func,

renderPanelTitle: PropTypes.func,
renderPanelContent: PropTypes.func,
}

public static defaultProps = {
Expand Down Expand Up @@ -153,7 +151,7 @@ class Accordion extends AutoControlledComponent<Extendable<AccordionProps>, any>

renderPanels = () => {
const children: any[] = []
const { panels, renderContent, renderTitle } = this.props
const { panels, renderPanelContent, renderPanelTitle } = this.props

_.each(panels, (panel, index) => {
const { content, title } = panel
Expand All @@ -163,13 +161,13 @@ class Accordion extends AutoControlledComponent<Extendable<AccordionProps>, any>
AccordionTitle.create(title, {
defaultProps: { active, index },
overrideProps: this.handleTitleOverrides,
render: renderTitle,
render: renderPanelTitle,
}),
)
children.push(
AccordionContent.create(content, {
defaultProps: { active },
render: renderContent,
render: renderPanelContent,
}),
)
})
Expand Down
70 changes: 2 additions & 68 deletions src/components/Attachment/Attachment.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as PropTypes from 'prop-types'
import * as React from 'react'
import * as _ from 'lodash'
import { Extendable, ShorthandValue } from '../../../types/utils'
import {
UIComponent,
customPropTypes,
Expand All @@ -9,7 +10,6 @@ import {
ChildrenComponentProps,
commonPropTypes,
} from '../../lib'
import { Extendable, ShorthandRenderFunction, ShorthandValue } from '../../../types/utils'
import Icon from '../Icon/Icon'
import Button from '../Button/Button'
import Text from '../Text/Text'
Expand All @@ -33,51 +33,6 @@ export interface AttachmentProps extends UIComponentProps, ChildrenComponentProp

/** Value indicating percent complete. */
progress?: string | number

/**
* A custom render function the action slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderAction?: ShorthandRenderFunction

/**
* A custom render function the description slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderDescription?: ShorthandRenderFunction

/**
* A custom render function the header slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderHeader?: ShorthandRenderFunction

/**
* A custom render function the icon slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderIcon?: ShorthandRenderFunction

/**
* A custom render function the progress slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderProgress?: ShorthandRenderFunction
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 much cleaner API!

}

/**
Expand All @@ -100,62 +55,41 @@ class Attachment extends UIComponent<Extendable<AttachmentProps>, any> {
header: customPropTypes.itemShorthand,
icon: customPropTypes.itemShorthand,
progress: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
renderAction: PropTypes.func,
renderDescription: PropTypes.func,
renderHeader: PropTypes.func,
renderIcon: PropTypes.func,
renderProgress: PropTypes.func,
}

renderComponent({ ElementType, classes, rest, styles, variables }) {
const {
header,
description,
icon,
action,
progress,
renderIcon,
renderHeader,
renderDescription,
renderAction,
renderProgress,
} = this.props
const { header, description, icon, action, progress } = this.props

return (
<ElementType {...rest} className={classes.root}>
{icon && (
<div className={classes.icon}>
{Icon.create(icon, {
defaultProps: { size: 'big' },
render: renderIcon,
})}
</div>
)}
{(header || description) && (
<div className={classes.content}>
{Text.create(header, {
defaultProps: { styles: styles.header },
render: renderHeader,
})}

{Text.create(description, {
defaultProps: { styles: styles.description },
render: renderDescription,
})}
</div>
)}
{action && (
<div className={classes.action}>
{Button.create(action, {
defaultProps: { iconOnly: true, text: true },
render: renderAction,
})}
</div>
)}
{!_.isNil(progress) &&
Slot.create('', {
defaultProps: { className: classes.progress },
render: renderProgress,
})}
</ElementType>
)
Expand Down
40 changes: 2 additions & 38 deletions src/components/Avatar/Avatar.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import * as PropTypes from 'prop-types'
import * as React from 'react'
import { Image, Label, Status } from '../../'

import { Extendable, ShorthandValue } from '../../../types/utils'
import {
createShorthandFactory,
customPropTypes,
UIComponent,
UIComponentProps,
commonPropTypes,
} from '../../lib'
import { Extendable, ShorthandRenderFunction, ShorthandValue } from '../../../types/utils'

export interface AvatarProps extends UIComponentProps {
/** Shorthand for the image. */
Expand All @@ -21,33 +20,6 @@ export interface AvatarProps extends UIComponentProps {
/** The name used for displaying the initials of the avatar if the image is not provided. */
name?: string

/**
* A custom render function the image slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderImage?: ShorthandRenderFunction

/**
* A custom render function the label slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderLabel?: ShorthandRenderFunction

/**
* A custom render function the status slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderStatus?: ShorthandRenderFunction

/** Size multiplier. */
size?: number

Expand Down Expand Up @@ -79,9 +51,6 @@ class Avatar extends UIComponent<Extendable<AvatarProps>, any> {
size: PropTypes.number,
status: customPropTypes.itemShorthand,
getInitials: PropTypes.func,
renderImage: PropTypes.func,
renderLabel: PropTypes.func,
renderStatus: PropTypes.func,
}

static defaultProps = {
Expand Down Expand Up @@ -110,8 +79,7 @@ class Avatar extends UIComponent<Extendable<AvatarProps>, any> {
}

renderComponent({ ElementType, classes, rest, styles, variables }) {
const { name, status, image, label, getInitials, renderImage, renderLabel, renderStatus } = this
.props as AvatarPropsWithDefaults
const { name, status, image, label, getInitials } = this.props as AvatarPropsWithDefaults

return (
<ElementType {...rest} className={classes.root}>
Expand All @@ -122,18 +90,15 @@ class Avatar extends UIComponent<Extendable<AvatarProps>, any> {
title: name,
styles: styles.image,
},
render: renderImage,
})}
{!image &&
!renderImage &&
Label.create(label || {}, {
defaultProps: {
content: getInitials(name),
circular: true,
title: name,
styles: styles.label,
},
render: renderLabel,
})}
{Status.create(status, {
defaultProps: {
Expand All @@ -143,7 +108,6 @@ class Avatar extends UIComponent<Extendable<AvatarProps>, any> {
borderWidth: variables.statusBorderWidth,
},
},
render: renderStatus,
})}
</ElementType>
)
Expand Down
20 changes: 2 additions & 18 deletions src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@ import Icon from '../Icon/Icon'
import Slot from '../Slot/Slot'
import { buttonBehavior } from '../../lib/accessibility'
import { Accessibility } from '../../lib/accessibility/types'
import {
ComponentEventHandler,
Extendable,
ShorthandRenderFunction,
ShorthandValue,
} from '../../../types/utils'
import { ComponentEventHandler, Extendable, ShorthandValue } from '../../../types/utils'
import ButtonGroup from './ButtonGroup'
import isFromKeyboard from '../../lib/isFromKeyboard'

Expand Down Expand Up @@ -70,15 +65,6 @@ export interface ButtonProps
/** A button can be formatted to show different levels of emphasis. */
primary?: boolean

/**
* A custom render function the icon slot.
*
* @param {React.ReactType} Component - The computed component for this slot.
* @param {object} props - The computed props for this slot.
* @param {ReactNode|ReactNodeArray} children - The computed children for this slot.
*/
renderIcon?: ShorthandRenderFunction

/** A button can be formatted to show only text in order to indicate some less-pronounced actions. */
text?: boolean

Expand Down Expand Up @@ -120,7 +106,6 @@ class Button extends UIComponent<Extendable<ButtonProps>, ButtonState> {
text: PropTypes.bool,
secondary: customPropTypes.every([customPropTypes.disallow(['primary']), PropTypes.bool]),
accessibility: PropTypes.func,
renderIcon: PropTypes.func,
}

public static defaultProps = {
Expand Down Expand Up @@ -163,15 +148,14 @@ class Button extends UIComponent<Extendable<ButtonProps>, ButtonState> {
}

public renderIcon = (variables, styles) => {
const { icon, iconPosition, content, renderIcon } = this.props
const { icon, iconPosition, content } = this.props

return Icon.create(icon, {
defaultProps: {
styles: styles.icon,
xSpacing: !content ? 'none' : iconPosition === 'after' ? 'before' : 'after',
variables: variables.icon,
},
render: renderIcon,
})
}

Expand Down
Loading