Skip to content

docs(fieldlabel/form): migrates docs to storybook #3165

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.

Sign up for GitHub

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 3 commits into from
Oct 2, 2024
Merged

docs(fieldlabel/form): migrates docs to storybook #3165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc01846
docs(fieldlabel): adds missing stories and cleans up test cases
marissahuysentruyt Sep 23, 2024
d6da6ff
docs(form): adds missing stories, controls, test cases
marissahuysentruyt Sep 25, 2024
6c3319c
chore(fieldlabel,form): pr edits
marissahuysentruyt Oct 1, 2024
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
82 changes: 81 additions & 1 deletion components/fieldlabel/stories/fieldlabel.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { disableDefaultModes } from "@spectrum-css/preview/modes";
import { isDisabled, isRequired, size } from "@spectrum-css/preview/types";
import { Sizes } from "@spectrum-css/preview/decorators";
import pkgJson from "../package.json";
import { FieldLabelGroup } from "./fieldlabel.test.js";
import { Template } from "./template.js";

/**
* The field label component is used along with inputs to display a label for that input.
Expand Down Expand Up @@ -39,15 +41,93 @@ export default {
alignment: "left",
isDisabled: false,
isRequired: false,
label: "Label",
},
parameters: {
packageJson: pkgJson,
},
};

/**
* By default, a field label has left-aligned text, and is a medium size. For field label text, use a short, catch-all description (1-3 words) of the information that a user needs to provide.
*
* The default position of a field label is above an input, but it can alternatively be positioned to the side. Visit the [form component](/docs/components-form--docs) to see examples of the field label with an input.

*/
export const Default = FieldLabelGroup.bind({});
Default.args = {
Default.args = {};

// ********* DOCS ONLY ********* //
/**
* Field labels come in four different sizes: small, medium, large, and extra-large. The medium size is the default and most frequently used option with medium-sized inputs. Use the other sizes sparingly; they should be used to create a hierarchy of importance within the page.
*
* Both small and medium field labels have the same font size, but different paddings when used as side labels.
*/
export const Sizing = (args, context) => Sizes({
Template,
withBorder: false,
withHeading: false,
...args,
}, context);
Sizing.tags = ["!dev"];
Sizing.parameters = {
chromatic: { disableSnapshot: true },
};

/**
* To render right-aligned field label text, apply the `.spectrum-FieldLabel--right` class to the field label.
*
*/
export const RightAligned = Template.bind({});
RightAligned.args = {
label: "Label",
alignment: "right",
customStyles: {
width: "72px",
},
};
RightAligned.tags = ["!dev"];
RightAligned.parameters = {
chromatic: { disableSnapshot: true },
};
RightAligned.storyName = "Right-aligned";

/**
* Field labels for required inputs can be marked with an asterisk at the end of the label. Optional inputs would then be understood to not have the asterisk. If using the asterisk icon, do not leave any space between the label text and the start of the `<svg>` element in the markup. Extra space should not be added in addition to the inline margin.
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There's a bit about the "necessity indicator" in the Spectrum guidelines that says you can use text like "(required)" or "(optional)". I didn't see that in the S2 field label Figma file, but it is the s2 field label migration PR.

I've left that off for now, but if we want to add it now, I certainly can. I believe I'd need to add an extra control when required is true, to swap between a text and icon option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, I was going to ask about this. I think it's fine to only show the asterisk. It might be worth mentioning that you can use text as well though? The docs site uses this wording, maybe something like this?

A Field label for a required field can display either the text “(required)”, or an asterisk.

*
* The field label for a required field can display either the text “(required)”, or an asterisk.
*/
export const Required = Template.bind({});
Required.args = {
isRequired: true,
};
Required.tags = ["!dev"];
Required.parameters = {
chromatic: { disableSnapshot: true },
};

/**
* When the field label is too long for the available horizontal space, it wraps to form another line.
*/
export const WrappingAndRequired = Template.bind({});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we want to keep this on the docs page, or is it enough to capture it in Chromatic? I think for the most part we've not been showing a wrapping story for components that have wrapping.

If we remove this story from the docs page, I'll keep the sentence about it wrapping, though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't mind it on the docs page, it certainly doesn't hurt to over-communicate in this case!

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an opinion but I wouldn't mind a control or something (like for instance restricting the width) in Storybook that would allow me to see how Field label wraps, not strictly necessary but maybe nice to have because otherwise the label can hold a lot of text before it'll start to wrap:
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm...this is a difficult one for me. It's definitely a little odd that this poor field label is just hanging out by itself, out of context. And I suppose that's my only hesitancy with adding a control, and we can use customStyles if needed (not that customStyles is shown on the docs page or in the controls table). Usually, a field label will be inside of some sort of container or form or something, right? If I go to textfield, the field label wraps at the width of the text field container. The wrapping doesn't seem to be an issue.

Screenshot 2024-10-01 at 5 23 15 PM

I don't have label controls set up for form right now, but if I go into the form.stories.js file and make up a long label, I don't actually see the text wrapping unless I use a ton of text like you did, I change the screen size, or again, pass some customStyles to form.

Screenshot 2024-10-01 at 5 29 29 PM

My gut says we don't need a control for this (namely because this component is just out of context and probably shouldn't be used on its own), but I'm open and can totally add it in if you think it's valuable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could also add something like this to the WrappingAndRequired story docs:

"Implementations can pass a maximum width to the field label component using customStyles."

WrappingAndRequired.args = {
label: "Label example with longer text that will wrap to the next line. And with an asterisk that marks it as required.",
isRequired: true,
customStyles: { width: "200px" },
};
WrappingAndRequired.tags = ["!dev"];
WrappingAndRequired.parameters = {
chromatic: { disableSnapshot: true },
};
WrappingAndRequired.storyName = "Wrapping and required";

export const Disabled = Template.bind({});
Disabled.args = {
isDisabled: true,
};
Disabled.tags = ["!dev"];
Disabled.parameters = {
chromatic: { disableSnapshot: true },
};

// ********* VRT ONLY ********* //
Expand Down
10 changes: 3 additions & 7 deletions components/fieldlabel/stories/fieldlabel.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@ export const FieldLabelGroup = Variants({
Template,
testData: [
{
testHeading: "Default"
},
{
testHeading: "Right alignment",
alignment: "right",
testHeading: "Default",
customStyles: { width: "200px" },
},
{
Expand All @@ -19,14 +15,14 @@ export const FieldLabelGroup = Variants({
},
{
testHeading: "Wrapped",
withStates: false,
label: "Label example with longer text that will wrap to the next line. And with an asterisk that marks it as required.",
label: "Label example with longer text that will wrap to the next line. Sometimes there is an asterisk that marks it as required.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't see an asterisk, but I wasn't supposed to, right?
image

Do we want to test it with the asterisk? Or with and without the asterisk?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct, we weren't actually testing for the asterisk, but I see how it's confusing now with that text! 🫠 I don't think it's really necessary to test the required state with the wrapping label, but I could be wrong. So just in case, I removed the withStates: false. I also added in ignore: ["Wrapped"] to the disabled state data, so let me know what you think!

It should show the wrapped optional, and wrapped required (with the asterisk) now. 6bbd57f

customStyles: { width: "200px" },
},
],
stateData: [
{
testHeading: "Disabled",
ignore: ["Wrapped"],
isDisabled: true,
customStyles: { width: "200px" },
},
Expand Down
165 changes: 116 additions & 49 deletions components/fieldlabel/stories/form.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,80 +1,147 @@
import { Template as Checkbox } from "@spectrum-css/checkbox/stories/template.js";
import { Template as Fieldgroup } from "@spectrum-css/fieldgroup/stories/template.js";
import { Template as Picker } from "@spectrum-css/picker/stories/template.js";
import { disableDefaultModes } from "@spectrum-css/preview/modes";
import { Template as Stepper } from "@spectrum-css/stepper/stories/template.js";
import { Template as TextField } from "@spectrum-css/textfield/stories/template.js";
import { Template } from "./form.template.js";
import pkgJson from "../package.json";
import { FormGroup } from "./form.test.js";

/**
* The form component is used for aligning multiple inputs and field groups within a form.
* The form component is used for aligning multiple inputs and field groups within a form. It provides structure and spacing for your form fields.
*/
export default {
title: "Form",
component: "Form",
argTypes: {
labelsAbove: {
name: "Labels above",
type: { name: "boolean" },
labelPosition: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed this to labelPosition because both text field and slider have similar controls called labelPosition.

name: "Label position",
description: "Position of the field label in relation to the input.",
type: { name: "string" },
table: {
type: { summary: "boolean" },
type: { summary: "string" },
category: "Component",
},
control: "boolean",
control: "select",
options: ["top", "side"],
},
fieldLabelAlignment: {
name: "Field label alignment",
type: { name: "string" },
table: {
type: { summary: "string" },
category: "Component",
},
control: "select",
options: ["left", "right"],
if: { arg: "labelPosition", eq: "side" },
},
items: { table: { disable: true } },
},
args: {
rootClass: "spectrum-Form",
labelsAbove: false,
labelPosition: "top",
items: [
{
label: "Company title",
id: "form-example-company",
content: [
(passthroughs, context) => TextField({
...passthroughs,
multiline: true,
name: "field",
}, context),
],
}, {
label: "Email address",
id: "form-example-email",
content: [
(passthroughs, context) => TextField({
...passthroughs,
type: "email",
name: "email",
}, context),
],
}, {
label: "Country",
id: "form-example-country",
content: [
(passthroughs, context) => Picker({
...passthroughs,
placeholder: "Select a country",
name: "country",
}, context),
],
}, {
label: "Interest",
id: "form-example-interests",
content: [
(passthroughs, context) => Fieldgroup({
layout: "horizontal",
items: [
Checkbox({
...passthroughs,
label: "Kittens",
customClasses: ["spectrum-FieldGroup-item"],
}, context),
Checkbox({
...passthroughs,
label: "Puppies",
customClasses: ["spectrum-FieldGroup-item"],
}, context),]
}),
],
},{
label: "Age",
id: "form-example-amount",
content: [
(passthroughs, context) => Stepper({
...passthroughs,
}, context),
]
}
],
},
parameters: {
packageJson: pkgJson,
},
};

/**
* A stacked layout with the labels above the form fields. The class `.spectrum-Form--labelPosition` is applied to the form itself. You do not need to apply a specific class to the field label because `.spectrum-FieldLabel--left` is applied to each to each [field label](/docs/components-field-label--docs) by default.
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

According to the field label docs, top labels are default, which is why the default story is changed. https://spectrum.adobe.com/page/field-label/#Label-position

When looking at textfield, if I toggle "displayFieldLabel," that also defaults to a top label.

*/
export const Default = FormGroup.bind({});
Default.args = {
items: [
{
label: "Company title",
id: "form-example-company",
content: [
(passthroughs, context) => TextField({
...passthroughs,
multiline: true,
name: "field",
}, context),
],
}, {
label: "Email address",
id: "form-example-email",
content: [
(passthroughs, context) => TextField({
...passthroughs,
type: "email",
name: "email",
}, context),
],
}, {
label: "Country",
id: "form-example-country",
content: [
(passthroughs, context) => Picker({
...passthroughs,
placeholder: "Select a country",
name: "country",
}, context),
],
}, {
label: "Amount",
id: "form-example-amount",
content: [
(passthroughs, context) => Stepper({
...passthroughs,
}, context),
]
}
],
Default.args = {};

/**
* A two column layout with left-aligned label text.
*/
export const LeftAlignedSideLabels = Template.bind({});
LeftAlignedSideLabels.args = {
...Default.args,
labelPosition: "side",
};
LeftAlignedSideLabels.tags = ["!dev"];
LeftAlignedSideLabels.parameters = {
chromatic: { disableSnapshot: true },
};
LeftAlignedSideLabels.storyName = "Left-aligned side labels";

/**
* A two column layout with right-aligned label text. `.spectrum-FieldLabel--right` is applied to each to each [field label](/docs/components-field-label--docs).
*/
export const RightAlignedSideLabels = Template.bind({});
RightAlignedSideLabels.args = {
...Default.args,
labelPosition: "side",
fieldLabelAlignment: "right",
};
RightAlignedSideLabels.tags = ["!dev"];
RightAlignedSideLabels.parameters = {
chromatic: { disableSnapshot: true },
};
RightAlignedSideLabels.storyName = "Right-aligned side labels";

// ********* VRT ONLY ********* //
export const WithForcedColors = FormGroup.bind({});
Expand Down
7 changes: 4 additions & 3 deletions components/fieldlabel/stories/form.template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import "../index.css";

export const Template = ({
rootClass = "spectrum-Form",
labelsAbove = false,
labelPosition = "top",
fieldLabelAlignment = "left",
customClasses = [],
customStyles = {},
id = getRandomId("form"),
Expand All @@ -20,7 +21,7 @@ export const Template = ({
<form
class=${classMap({
[rootClass]: true,
[`${rootClass}--labelsAbove`]: labelsAbove,
[`${rootClass}--labelsAbove`]: labelPosition === "top",
...customClasses.reduce((a, c) => ({ ...a, [c]: true }), {}),
})}
id=${ifDefined(id)}
Expand All @@ -36,7 +37,7 @@ export const Template = ({
${when(label, () => FieldLabel({
label,
forInput: item.id,
alignment: labelsAbove ? undefined : "left",
alignment: labelPosition === "side" ? fieldLabelAlignment : undefined,
}, context))}
<div class=${classMap({
[`${rootClass}-itemField`]: true,
Expand Down
13 changes: 10 additions & 3 deletions components/fieldlabel/stories/form.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,17 @@ import { Template } from "./form.template";
export const FormGroup = Variants({
Template,
testData: [
{},
{
testHeading: "Labels above",
labelsAbove: true,
testHeading: "Default",
},
{
testHeading: "Side labels with left-aligned text",
labelPosition: "side",
},
{
testHeading: "Side labels with right-aligned text",
labelPosition: "side",
fieldLabelAlignment: "right",
}
],
});
Loading