-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[Forms] Add <Label> and <FormGroup> examples #2433
Changes from 3 commits
b10c39e
d7da44a
2043889
0691fd3
41e70d1
d652cd6
7a0c5cc
613ef21
ed08e8d
4817b11
a53bd4d
4cf9bea
0fa65f4
9fa56c6
cedb5bb
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 |
---|---|---|
|
@@ -4,6 +4,17 @@ Form groups support more complex form controls than [simple labels](#core/compon | |
such as [control groups](#core/components/forms/control-group) or [`NumericInput`](#core/components/forms/numeric-input). | ||
They also support additional helper text to aid with user navigation. | ||
|
||
@reactExample FormGroupExample | ||
|
||
@## JavaScript API | ||
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. we're still in the habit of CSS API appearing before JS API so there's no good reason to move only this one. |
||
|
||
The `FormGroup` component is available in the __@blueprintjs/core__ package. | ||
Make sure to review the [getting started docs for installation info](#blueprint/getting-started). | ||
|
||
This component is a simple wrapper around the CSS API that abstracts away the HTML complexity. | ||
|
||
@interface IFormGroupProps | ||
|
||
@## CSS API | ||
|
||
- Link each label to its respective control element with a `for={#id}` attribute on the `<label>` and | ||
|
@@ -17,23 +28,3 @@ Similar to labels, nested controls need to be styled separately. | |
- Add `.@ns-large` to `.@ns-form-group` to align the label when used with large inline Blueprint controls. | ||
|
||
@css form-group | ||
|
||
@## JavaScript API | ||
|
||
The `FormGroup` component is available in the __@blueprintjs/core__ package. | ||
Make sure to review the [getting started docs for installation info](#blueprint/getting-started). | ||
|
||
This component is a simple wrapper around the CSS API that abstracts away the HTML complexity. | ||
|
||
```tsx | ||
<FormGroup | ||
helperText="Helper text with details..." | ||
label="Label A" | ||
labelFor="text-input" | ||
requiredLabel={true} | ||
> | ||
<input id="text-input" placeholder="Placeholder text" /> | ||
</FormGroup> | ||
``` | ||
|
||
@interface IFormGroupProps |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,14 +54,14 @@ export class FormGroup extends React.PureComponent<IFormGroupProps, {}> { | |
* | ||
* Defaults to `<span class={Classes.TEXT_MUTED}>(required)</span>`. | ||
*/ | ||
public static DEFAULT_REQUIRED_CONTENT = <span className={Classes.TEXT_MUTED}>(required)</span>; | ||
public static DEFAULT_REQUIRED_CONTENT = <span className={Classes.TEXT_MUTED}> (required)</span>; | ||
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. this doesn't fix the issue if the user supplies their own required label. instead add |
||
|
||
public render() { | ||
const { children, label, labelFor } = this.props; | ||
return ( | ||
<div className={this.getClassName()}> | ||
<label className={Classes.LABEL} htmlFor={labelFor}> | ||
{label} | ||
<span className={Classes.LABEL_TEXT}>{label}</span> | ||
{this.maybeRenderRequiredLabel()} | ||
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. better to add a space between the two elements here, or via margin in CSS. |
||
</label> | ||
<div className={Classes.FORM_CONTENT}> | ||
|
@@ -87,7 +87,7 @@ export class FormGroup extends React.PureComponent<IFormGroupProps, {}> { | |
|
||
private maybeRenderRequiredLabel() { | ||
const { requiredLabel } = this.props; | ||
return requiredLabel === true ? FormGroup.DEFAULT_REQUIRED_CONTENT : requiredLabel; | ||
return requiredLabel === true ? FormGroup.DEFAULT_REQUIRED_CONTENT : <span> {requiredLabel}</span>; | ||
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. see comment above |
||
} | ||
|
||
private maybeRenderHelperText() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,33 @@ | ||
@# Labels | ||
|
||
Labels enhance the usability of your forms. | ||
|
||
<div class="@ns-callout @ns-intent-success @ns-icon-comparison"> | ||
<h4 class="@ns-callout-title">Simple labels vs. form groups</h4> | ||
<p>Blueprint provides two ways of connecting label text to control fields, depending on the complexity of the control.</p> | ||
<p>Simple labels are a basic way to connect a label with a single control.</p> | ||
<p>Form groups support more complex control layouts but require more markup to maintain consistent visuals.</p> | ||
</div> | ||
|
||
Labels enhance the usability of your forms. | ||
|
||
@reactExample LabelExample | ||
|
||
@## JavaScript API | ||
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. again with the moving JS before CSS |
||
|
||
The `Label` component is available in the __@blueprintjs/core__ package. Make sure to review the [getting started docs for installation info](#blueprint/getting-started). | ||
|
||
This component is a simple wrapper around the corresponding CSS API. It supports the full range of HTML props. | ||
|
||
```tsx | ||
<Label | ||
helperText="Helper text with details..." | ||
text="Label A" | ||
> | ||
<input className="@ns-input" id="text-input" placeholder="Placeholder text" /> | ||
</Label> | ||
``` | ||
|
||
@interface ILabelProps | ||
|
||
@## CSS API | ||
|
||
@### Simple labels | ||
|
@@ -33,19 +52,3 @@ must add the `:disabled` attribute directly to any nested elements to disable th | |
|
||
@css label-disabled | ||
|
||
@## JavaScript API | ||
|
||
The `Label` component is available in the __@blueprintjs/core__ package. Make sure to review the [getting started docs for installation info](#blueprint/getting-started). | ||
|
||
This component is a simple wrapper around the corresponding CSS API. It supports the full range of HTML props. | ||
|
||
```tsx | ||
<Label | ||
helperText="Helper text with details..." | ||
text="Label A" | ||
> | ||
<input className="@ns-input" id="text-input" placeholder="Placeholder text" /> | ||
</Label> | ||
``` | ||
|
||
@interface ILabelProps |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
/* | ||
* Copyright 2016 Palantir Technologies, Inc. All rights reserved. | ||
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. 2018 |
||
* | ||
* Licensed under the terms of the LICENSE file distributed with this project. | ||
*/ | ||
|
||
import * as React from "react"; | ||
|
||
import { FormGroup, InputGroup, Switch } from "@blueprintjs/core"; | ||
import { BaseExample, handleBooleanChange } from "@blueprintjs/docs-theme"; | ||
|
||
export interface IFormGroupExampleState { | ||
disabled: boolean; | ||
helperText: boolean; | ||
inline: boolean; | ||
} | ||
|
||
export class FormGroupExample extends BaseExample<IFormGroupExampleState> { | ||
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. rewrite for new example syntax |
||
public state: IFormGroupExampleState = { | ||
disabled: false, | ||
helperText: false, | ||
inline: false, | ||
}; | ||
|
||
protected className = "docs-label-example"; | ||
|
||
private handleDisabledChange = handleBooleanChange(disabled => this.setState({ disabled })); | ||
private handleHelperTextChange = handleBooleanChange(helperText => this.setState({ helperText })); | ||
private handleInlineChange = handleBooleanChange(inline => this.setState({ inline })); | ||
|
||
protected renderExample() { | ||
const { disabled, helperText, inline } = this.state; | ||
return ( | ||
<FormGroup | ||
disabled={disabled} | ||
helperText={helperText ? "Helper text with details..." : undefined} | ||
inline={inline} | ||
label="Label" | ||
requiredLabel={true} | ||
labelFor="text-input" | ||
> | ||
<InputGroup id="text-input" placeholder="Placeholder text" disabled={disabled} /> | ||
</FormGroup> | ||
); | ||
} | ||
|
||
protected renderOptions() { | ||
const { disabled, helperText, inline } = this.state; | ||
return [ | ||
[ | ||
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. fragment!!! |
||
<Switch key="inline" label="Inline" checked={inline} onChange={this.handleInlineChange} />, | ||
<Switch | ||
key="helperText" | ||
label="Show helper text" | ||
checked={helperText} | ||
onChange={this.handleHelperTextChange} | ||
/>, | ||
<Switch key="disabled" label="Disabled" checked={disabled} onChange={this.handleDisabledChange} />, | ||
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. put this switch first please (alph) |
||
], | ||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/* | ||
* Copyright 2016 Palantir Technologies, Inc. All rights reserved. | ||
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. 2018 |
||
* | ||
* Licensed under the terms of the LICENSE file distributed with this project. | ||
*/ | ||
|
||
import * as React from "react"; | ||
|
||
import { InputGroup, Label, Switch } from "@blueprintjs/core"; | ||
import { BaseExample, handleBooleanChange } from "@blueprintjs/docs-theme"; | ||
|
||
export interface ILabelExampleState { | ||
disabled: boolean; | ||
helperText: boolean; | ||
inline: boolean; | ||
} | ||
|
||
export class LabelExample extends BaseExample<ILabelExampleState> { | ||
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. given #2460, do we really want to add a 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. I'd say it doesn't hurt to have one for now, helps with testing and comparing with 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. rewrite to use new |
||
public state: ILabelExampleState = { | ||
disabled: false, | ||
helperText: false, | ||
inline: false, | ||
}; | ||
|
||
protected className = "docs-label-example"; | ||
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. these are not necessary. let's only add them when required for custom styles |
||
|
||
private handleDisabledChange = handleBooleanChange(disabled => this.setState({ disabled })); | ||
private handleHelperTextChange = handleBooleanChange(helperText => this.setState({ helperText })); | ||
private handleInlineChange = handleBooleanChange(inline => this.setState({ inline })); | ||
|
||
protected renderExample() { | ||
const { disabled, helperText, inline, ...labelProps } = this.state; | ||
return ( | ||
<Label | ||
disabled={disabled} | ||
helperText={helperText ? "(required)" : undefined} | ||
inline={inline} | ||
text="Label" | ||
{...labelProps} | ||
> | ||
<InputGroup type="text" placeholder="Input with icon" leftIcon="text-highlight" disabled={disabled} /> | ||
</Label> | ||
); | ||
} | ||
|
||
protected renderOptions() { | ||
const { disabled, helperText, inline } = this.state; | ||
return [ | ||
[ | ||
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. fragment!!! |
||
<Switch key="inline" label="Inline" checked={inline} onChange={this.handleInlineChange} />, | ||
<Switch | ||
key="helperText" | ||
label="Show helper text" | ||
checked={helperText} | ||
onChange={this.handleHelperTextChange} | ||
/>, | ||
<Switch key="disabled" label="Disabled" checked={disabled} onChange={this.handleDisabledChange} />, | ||
], | ||
]; | ||
} | ||
} |
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.
why not just use
<strong>
tags if all you want is bold? it's both semantic and requires no additional styles or classes.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.
cause if we change this in the future, we'll also have to change the tag. poor maintanability
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.
but that tag isn't going anywhere! it's the semantic browser way of saying "make this text stronger," which is exactly what you're trying to do here.
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.
please try it. i'm quite confident it'll work beautifully and we won't have to add another
-text
class for one style rule.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.
what I meant is that I don't like tying a visual to a specific tag. if in the future I want to get rid of bold and do another visual style, I'll have to change the tag because
<strong>
will no longer make sense.did it anyway, kind of a bummer tho cause I still need to override the
font-weight
of the<strong>
(default is 700, we want 600)