[Forms] Add <Label> and <FormGroup> examples #2433

llorca · 2018-04-27T23:19:55Z

fixes #2416

add JS examples for Label and FormGroup

blueprint-bot · 2018-04-27T23:23:34Z

add space between form group label and requiredLabel

Preview: documentation | landing | table

giladgray · 2018-04-27T23:53:38Z

packages/core/src/components/forms/_form-group.scss

@@ -10,7 +10,7 @@ Form groups
 Markup:
 <div class="#{$ns}-form-group">
  <label class="#{$ns}-label" for="example-form-group-input-a">
-    Label A
+    <span class="#{$ns}-label-text">Label A</span>


why not just use  tags if all you want is bold? it's both semantic and requires no additional styles or classes.

cause if we change this in the future, we'll also have to change the tag. poor maintanability

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.

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.

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  will no longer make sense.

did it anyway, kind of a bummer tho cause I still need to override the font-weight of the  (default is 700, we want 600)

blueprint-bot · 2018-04-28T00:13:38Z

change label-text to a strong tag

Preview: documentation | landing | table

giladgray · 2018-04-28T00:09:51Z

packages/core/src/components/forms/_label.scss

@@ -31,6 +31,10 @@ label.#{$ns}-label {
  display: block;
  margin: 0 0 ($pt-grid-size * 1.5);

+  strong {


this does not belong here. 1) it should already be true. 2) if not, it should be applied such that all strong tags look the same in blueprint.

so 1) not true yet 2) not that simple. the boldness is a function of the font size.

since 600 is the most common "bold" weight for 14px text (our default UI font size), I suppose it's fair to default  to 600 weight. I can then remove this rule, 👍 or 👎 ?

sounds good, I'll address as soon as I can!

giladgray · 2018-04-28T00:10:23Z

packages/core/src/components/forms/form-group.md

@@ -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


we're still in the habit of CSS API appearing before JS API so there's no good reason to move only this one.

giladgray · 2018-04-28T00:10:55Z

packages/core/src/components/forms/formGroup.tsx

@@ -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>;


this doesn't fix the issue if the user supplies their own required label. instead add {" "} in the JSX itself between the two elements (at the end of line 64).

giladgray · 2018-04-28T00:11:28Z

packages/core/src/components/forms/label.md

+
+@reactExample LabelExample
+
+@## JavaScript API


again with the moving JS before CSS

giladgray · 2018-04-28T00:11:59Z

packages/core/src/components/forms/formGroup.tsx

@@ -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>;


see comment above

giladgray · 2018-04-28T00:12:50Z

packages/docs-app/src/examples/core-examples/formGroupExample.tsx

+                    checked={helperText}
+                    onChange={this.handleHelperTextChange}
+                />,
+                <Switch key="disabled" label="Disabled" checked={disabled} onChange={this.handleDisabledChange} />,


put this switch first please (alph)

giladgray · 2018-04-28T00:12:56Z

packages/docs-app/src/examples/core-examples/labelExample.tsx

@@ -0,0 +1,61 @@
+/*
+ * Copyright 2016 Palantir Technologies, Inc. All rights reserved.


giladgray · 2018-04-28T00:13:00Z

packages/docs-app/src/examples/core-examples/formGroupExample.tsx

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2016 Palantir Technologies, Inc. All rights reserved.


blueprint-bot · 2018-04-28T00:22:47Z

move css back on top of JS

Preview: documentation | landing | table

blueprint-bot · 2018-04-28T00:25:57Z

correct copyright year

Preview: documentation | landing | table

giladgray · 2018-04-30T22:23:44Z

@llorca test failures because you changed the rendered markup

blueprint-bot · 2018-05-04T20:41:15Z

remove strong rule from label css

Preview: documentation | landing | table

llorca · 2018-05-04T20:43:21Z

would need your help to fix tests, I'm not familiar with how they work

blueprint-bot · 2018-05-04T20:45:29Z

make default strong 600 weight

Preview: documentation | landing | table

blueprint-bot · 2018-05-04T21:28:48Z

typo

Preview: documentation | landing | table

blueprint-bot · 2018-05-14T23:40:46Z

merge conflicts

Preview: documentation | landing | table

giladgray

this PR needs some more thought given #2460

giladgray · 2018-05-15T17:40:55Z

packages/core/src/components/forms/_label.scss

@@ -77,6 +77,29 @@ label.#{$ns}-label {
  Disabled labels

  Markup:
+<<<<<<< HEAD


merge conflict

giladgray · 2018-05-15T17:42:30Z

packages/docs-app/src/examples/core-examples/labelExample.tsx

+        inline: false,
+    };
+
+    protected className = "docs-label-example";


these are not necessary. let's only add them when required for custom styles

giladgray · 2018-05-15T17:43:07Z

packages/docs-app/src/examples/core-examples/labelExample.tsx

+    inline: boolean;
+}
+
+export class LabelExample extends BaseExample<ILabelExampleState> {


given #2460, do we really want to add a Label example if we're gonna kill it for FormGroup soon?

I'd say it doesn't hurt to have one for now, helps with testing and comparing with FormGroup. will be easy enough to remove it if we really kill Label

rewrite to use new Example component

blueprint-bot · 2018-05-15T18:56:19Z

address comments

Preview: documentation | landing | table

giladgray · 2018-05-15T20:40:10Z

@llorca tests failing due to added space.

blueprint-bot · 2018-05-16T00:05:40Z

add show/hide required label in form group example

Preview: documentation | landing | table

llorca · 2018-05-16T23:11:42Z

still need to fix tests
after working with example options, I’m having second guesses about broadly making all labels bold, but I’d still want it in specific cases, so I want to allow consumers to apply CSS to it. I’m thinking removing  and add a  with Classes.LABEL_TEXT like I had originally
- pros: proper targetable classname, and we can omit the new  from the CSS markup (tho it would come for free with the React component)
- cons: ??

giladgray · 2018-05-16T23:22:01Z

 seems more than sufficient here: if you want a bold label, simply wrap the label text in a  tag and voila it's perfect.

llorca · 2018-05-16T23:24:10Z

ok fair!

blueprint-bot · 2018-05-16T23:32:23Z

remove all strongs

Preview: documentation | landing | table

giladgray · 2018-05-16T23:33:57Z

packages/core/src/components/forms/formGroup.tsx

-        return requiredLabel === true ? FormGroup.DEFAULT_REQUIRED_CONTENT : requiredLabel;
+        if (requiredLabel !== false) {
+            const label = requiredLabel === true ? FormGroup.DEFAULT_REQUIRED_CONTENT : requiredLabel;
+            return <span className={Classes.TEXT_MUTED}> {label}</span>;


this is not the correct place to add the space, and it's causing the tests to fail. please remove.

giladgray · 2018-05-16T23:34:13Z

packages/core/src/components/forms/formGroup.tsx

        return (
-            <div className={this.getClassName()}>
+            <div className={classes}>
                <label className={Classes.LABEL} htmlFor={labelFor}>
                    {label}
                    {this.maybeRenderRequiredLabel()}


better to add a space between the two elements here, or via margin in CSS. {label}{" "}{this...}

giladgray · 2018-05-16T23:35:03Z

packages/docs-app/src/examples/core-examples/formGroupExample.tsx

+    protected renderOptions() {
+        const { disabled, helperText, inline, requiredLabel } = this.state;
+        return [
+            [


fragment!!!

giladgray · 2018-05-16T23:35:16Z

packages/docs-app/src/examples/core-examples/labelExample.tsx

+    protected renderOptions() {
+        const { disabled, helperText, inline } = this.state;
+        return [
+            [


fragment!!!

giladgray · 2018-05-16T23:35:25Z

packages/docs-app/src/examples/core-examples/formGroupExample.tsx

+    requiredLabel: boolean;
+}
+
+export class FormGroupExample extends BaseExample<IFormGroupExampleState> {


rewrite for new example syntax

giladgray · 2018-05-16T23:35:39Z

packages/docs-app/src/examples/core-examples/labelExample.tsx

+    inline: boolean;
+}
+
+export class LabelExample extends BaseExample<ILabelExampleState> {


rewrite to use new Example component

giladgray · 2018-05-16T23:36:14Z

gonna have to change the code or a test to make this work.

giladgray · 2018-05-29T18:47:26Z

closing as stale. let's do all this as part of #2460

Antoine Llorca added 3 commits April 27, 2018 16:16

add label and form group JS examples

b10c39e

bolder labels

d7da44a

add space between form group label and requiredLabel

2043889

llorca requested a review from giladgray April 27, 2018 23:19

giladgray reviewed Apr 27, 2018

View reviewed changes

change label-text to a strong tag

0691fd3

giladgray suggested changes Apr 28, 2018

View reviewed changes

Antoine Llorca added 3 commits April 27, 2018 17:17

move css back on top of JS

41e70d1

add space between label and required label

d652cd6

correct copyright year

7a0c5cc

Antoine Llorca added 2 commits May 4, 2018 13:35

remove strong rule from label css

613ef21

make default strong 600 weight

ed08e8d

typo

4817b11

merge conflicts

a53bd4d

giladgray suggested changes May 15, 2018

View reviewed changes

address comments

4cf9bea

Antoine Llorca added 2 commits May 15, 2018 16:58

move space within span

0fa65f4

add show/hide required label in form group example

9fa56c6

llorca requested a review from giladgray May 15, 2018 23:59

llorca changed the title ~~[Forms] Bolder labels~~ [Forms] Add <Label> and <FormGroup> examples May 16, 2018

remove all strongs

cedb5bb

giladgray suggested changes May 16, 2018

View reviewed changes

giladgray closed this May 29, 2018

giladgray deleted the al/label-bold branch October 14, 2018 17:02

		@@ -0,0 +1,61 @@
		/*
		* Copyright 2016 Palantir Technologies, Inc. All rights reserved.

		@@ -0,0 +1,62 @@
		/*
		* Copyright 2016 Palantir Technologies, Inc. All rights reserved.

[Forms] Add <Label> and <FormGroup> examples #2433

[Forms] Add <Label> and <FormGroup> examples #2433

Conversation

llorca commented Apr 27, 2018 • edited by giladgray Loading

fixes #2416

blueprint-bot commented Apr 27, 2018

add space between form group label and requiredLabel

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueprint-bot commented Apr 28, 2018

change label-text to a strong tag

Choose a reason for hiding this comment

llorca Apr 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueprint-bot commented Apr 28, 2018

move css back on top of JS

blueprint-bot commented Apr 28, 2018

correct copyright year

giladgray commented Apr 30, 2018

blueprint-bot commented May 4, 2018

remove strong rule from label css

llorca commented May 4, 2018

blueprint-bot commented May 4, 2018

make default strong 600 weight

blueprint-bot commented May 4, 2018

typo

blueprint-bot commented May 14, 2018

merge conflicts

giladgray left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueprint-bot commented May 15, 2018

address comments

giladgray commented May 15, 2018

blueprint-bot commented May 16, 2018

add show/hide required label in form group example

llorca commented May 16, 2018 • edited Loading

giladgray commented May 16, 2018

llorca commented May 16, 2018

blueprint-bot commented May 16, 2018

remove all strongs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giladgray commented May 16, 2018

giladgray commented May 29, 2018

llorca commented Apr 27, 2018 •

edited by giladgray

Loading

llorca Apr 28, 2018 •

edited

Loading

llorca commented May 16, 2018 •

edited

Loading