-
Notifications
You must be signed in to change notification settings - Fork 2.9k
add onRenderDescription to TextField #4588
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
Changes from 4 commits
3d65b14
50eaa79
b6808b2
5f6ad33
c15acaf
383f313
a65ef5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "office-ui-fabric-react", | ||
| "comment": "add onRenderDescription to TextField", | ||
| "type": "minor" | ||
| } | ||
| ], | ||
| "packageName": "office-ui-fabric-react", | ||
| "email": "chrismo@microsoft.com" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,7 +150,8 @@ export class TextField extends BaseComponent<ITextFieldProps, ITextFieldState> i | |
| onRenderAddon = this._onRenderAddon, // @deprecated | ||
| onRenderPrefix = this._onRenderPrefix, | ||
| onRenderSuffix = this._onRenderSuffix, | ||
| onRenderLabel = this._onRenderLabel | ||
| onRenderLabel = this._onRenderLabel, | ||
| onRenderDescription = this._onRenderDescription | ||
| } = this.props; | ||
| const { isFocused } = this.state; | ||
| const errorMessage = this._errorMessage; | ||
|
|
@@ -192,7 +193,7 @@ export class TextField extends BaseComponent<ITextFieldProps, ITextFieldState> i | |
| </div> | ||
| { this._isDescriptionAvailable && | ||
| <span id={ this._descriptionId }> | ||
| { description && <span className={ css('ms-TextField-description', styles.description) }>{ description }</span> } | ||
| { onRenderDescription(this.props, this._onRenderDescription) } | ||
| { errorMessage && | ||
| <div aria-live='assertive'> | ||
| <DelayedRender> | ||
|
|
@@ -304,6 +305,14 @@ export class TextField extends BaseComponent<ITextFieldProps, ITextFieldState> i | |
| return null; | ||
| } | ||
|
|
||
| private _onRenderDescription = (props: ITextFieldProps): JSX.Element | null => { | ||
| const { description } = props; | ||
| if (description) { | ||
| return (<span className={ css('ms-TextField-description', styles.description) }>{ description }</span>); | ||
| } | ||
| return null; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The following code will do the same return description && <span....>
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Slight challenge here was that we'd have to add string to the return type, because of falsey empty strings. |
||
| } | ||
|
|
||
| // @deprecated | ||
| private _onRenderAddon(props: ITextFieldProps): JSX.Element { | ||
| const { addonString } = props; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,6 +87,11 @@ export interface ITextFieldProps extends React.AllHTMLAttributes<HTMLInputElemen | |
| */ | ||
| description?: string; | ||
|
|
||
| /** | ||
| * Optional custom renderer for the description | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
nit: . at the end of the sentence.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| */ | ||
| onRenderDescription?: IRenderFunction<ITextFieldProps>; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please add this along with the rest of the onRender APIs below
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added periods to the descriptions of all onRender.... functions |
||
|
|
||
| /** | ||
| * @deprecated | ||
| * Deprecated; use prefix instead. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import * as React from 'react'; | ||
| import { TextField, ITextFieldProps } from 'office-ui-fabric-react/lib/TextField'; | ||
| import './TextField.Examples.scss'; | ||
|
|
||
| export class TextFieldOnRenderDescriptionExample extends React.Component<{}, {}> { | ||
| public render() { | ||
| return ( | ||
| <div className='docs-TextFieldExample'> | ||
| <TextField | ||
| description={ 'A custom description that appends a link.' } | ||
| onRenderDescription={ this._onRenderDescription } | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| private _onRenderDescription = (props: ITextFieldProps): JSX.Element => { | ||
| return ( | ||
| <div> | ||
| { props.description }{ ' ' } | ||
| <a href='#' onClick={ this._onLinkClick } >Link</a> | ||
| </div>); | ||
| } | ||
|
|
||
| private _onLinkClick = (e: React.MouseEvent<HTMLElement>) => { | ||
| e.preventDefault(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manishgarg1 I've updated both
_onRenderDescriptionand_onRenderLabel.A bit different than you suggested though. Rather than replacing
nullwithundefined, I was able to remove null from the return type by moving the guard into the render method.(I wasn't sure for the motivation behind your suggestion, so please let me know if I missed the point.)
The reason I did it this way was because changing
nulltoundefinedon the return type resulted in the following error at theonRenderLabelinvokation, which I don't yet understand.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manishgarg1 On further consideration, I don't think what I did will work for us. We'd want the provided onRenderDescription to be invoked even when description is absent. I've reverted this last change.
If you still think
nullshould be changed toundefined, can you please help me understand the motivation? And suggest a course of action given the error that I encountered above?