Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions docs/validations.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ Component props that are not supported if the opt-in flag is set to true are lab
The legacy validation pattern is still available for use, but we recommend using the new validation pattern for a more consistent and user-friendly experience.
This pattern uses tooltips to provide feedback to users about the state of their input.

The `Textbox` component is used below for documentation purposes, but it is recommend to use the new `TextInput` component instead which only uses the new validation pattern
by default.

#### States

Each input component that supports validations accepts the following props - `error`, `warning` and `info`.
Expand Down Expand Up @@ -117,7 +120,7 @@ For more information on how to use React Hook Form, please refer to the [React H
import { useForm, SubmitHandler, Controller } from 'react-hook-form';
import Button from 'carbon-react/lib/components/button';
import Form from 'carbon-react/lib/components/form';
import Textbox from 'carbon-react/lib/components/textbox';
import TextInput from 'carbon-react/lib/components/textbox/__next__';

interface FormValues {
name: string;
Expand Down Expand Up @@ -146,7 +149,7 @@ const MyForm = () => {
required: 'Name is required',
}}
render={({ field: { onChange, onBlur, value }, fieldState }) => (
<Textbox
<TextInput
label="Name"
required
value={value}
Expand All @@ -167,4 +170,3 @@ export default MyForm;

For legacy projects who still use Formik, please refer to our previous documentation on using
[Formik](https://carbon.sage.com/v/154.0.0/index.html?path=/docs/documentation-validations--docs#formik-overview) with Carbon components.

5 changes: 3 additions & 2 deletions docs/validations.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from "react";
import { Meta, StoryObj } from "@storybook/react";

import Textbox from "../src/components/textbox";
import TextInput from "../src/components/textbox/__next__";
import { RadioButton, RadioButtonGroup } from "../src/components/radio-button";
import { Checkbox, CheckboxGroup } from "../src/components/checkbox";
import CarbonProvider from "../src/components/carbon-provider";
Expand Down Expand Up @@ -134,15 +135,15 @@ export const ValidationRedesign: StoryObj = () => {
<RequiredFieldsIndicator mb={2}>
Fill in all fields marked with
</RequiredFieldsIndicator>
<Textbox
<TextInput
Copy link
Contributor

Choose a reason for hiding this comment

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

comment: we need to consider what we need to still capture for the old textbox visual regression wise, again worth discussing with the rest of the team in slack

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed - maybe use one TextInput and one Textbox to cover off both?

label="Textbox"
inputHint="Hint text"
value={state}
onChange={(e) => setState(e.target.value)}
required
error="Error Message (Fix is required)"
/>
<Textbox
<TextInput
label="Textbox"
inputHint="Hint text"
value={state}
Expand Down
1 change: 1 addition & 0 deletions src/__internal__/validation-message/__next__/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from "./validation-message.component";
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React from "react";
import StyledValidationMessage from "./validation-message.style";
import tagComponent, {
TagProps,
} from "../../../__internal__/utils/helpers/tags";

export interface ValidationMessageProps extends TagProps {
/**
* If true, displays error styling. If passed as a valid string, displays error styling
* and the message as children.
*/
error?: boolean | string;
/** Id of the component, to be used for accessibility purposes */
id?: string;
/**
* If true, displays warning styling. If passed as a valid string, displays warning styling
* and the message as children.
*/
warning?: boolean | string;
/** If true, uses large font size */
isLarge?: boolean;
}

const ValidationMessage = ({
error,
id,
warning,
"data-element": dataElement,
"data-role": dataRole = "validation-message",
isLarge,
}: ValidationMessageProps) => {
const validation = error || warning;
const isStringValidation = typeof validation === "string";

return isStringValidation ? (
<StyledValidationMessage
id={id}
isError={Boolean(error)}
{...tagComponent("validation-message", {
"data-element": dataElement,
"data-role": dataRole,
})}
isLarge={isLarge}
>
{validation}
</StyledValidationMessage>
) : null;
};

export default ValidationMessage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import styled, { css } from "styled-components";

interface StyledValidationMessageProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (non-blocking): these won;'t be added to the DOM element so up to you

interface StyledValidationMessageProps {
  $isError?: boolean;
  $isDarkBackground?: boolean;
  $isLarge?: boolean;
}

isError?: boolean;
isDarkBackground?: boolean;
isLarge?: boolean;
}

const StyledValidationMessage = styled.span<StyledValidationMessageProps>`
${({ isError, isLarge }) => {
return css`
display: flex;
align-items: center;
align-content: center;
align-self: stretch;
flex-wrap: wrap;
margin: 0px;
margin: 0px;
color: ${isError
? "var(--input-validation-label-error)"
: "var(--input-validation-label-warn)"};
font-family: var(--fontFamiliesDefault);
Copy link
Contributor

Choose a reason for hiding this comment

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

question: why aren't we using the new tokens for the font?

Copy link
Contributor

Choose a reason for hiding this comment

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

comment: The font tokens contain everything already as far as I know so you could just set font unless I'm missing something, you might need to pick the relevant one based on size and error vs warning though (worth checking tokens with DS/UX though)

   --global-font-static-comp-medium-xs: 500 13px/1.5 Sage UI;
  --global-font-static-comp-medium-s: 500 14px/1.5 Sage UI;
  --global-font-static-comp-medium-m: 500 14px/1.5 Sage UI;
  --global-font-static-comp-medium-l: 500 16px/1.5 Sage UI;
  --global-font-static-comp-regular-xs: 400 13px/1.5 Sage UI;
  --global-font-static-comp-regular-s: 400 14px/1.5 Sage UI;
  --global-font-static-comp-regular-m: 400 14px/1.5 Sage UI;
  --global-font-static-comp-regular-l: 400 16px/1.5 Sage UI;
font-family: var(--fontFamiliesDefault);
font-size: ${isLarge
        ? "var(--global-font-static-body-regular-l)"
        : "var(--global-font-static-body-regular-m)"};

font-style: normal;
font-weight: ${isError ? "500" : "400"};
line-height: 150%;

can be

${isLarge && css`
  font: ${isError ? "var(--global-font-static-comp-medium-l)" : ""var(--global-font-static-comp-regular-l)"};
`}
${!isLarge && css`
     font: ${isError ? "var(--global-font-static-comp-medium-m)" : ""var(--global-font-static-comp-regular-m)"};
`}

Copy link
Contributor

Choose a reason for hiding this comment

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

@edleeks87 i don't believe there's any nice way to show hint text inline. When switching it on within figma it looks identical to Tom's implementation. TBF inline is generally discouraged. Where is see it used, it never has hint text enabled. e.g for quick filtering on top of tables. I think we have 2 options:

  1. Not allow hint text on inline and restrict from a code POV
  2. just leave as is knowing that its never going to be used and doesn't matter how it therefore appears.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think personally we should keep inline input hints, if we're providing the capacity to make labels inline via labelInline, it makes sense to have everything available in both label states. We've made accommodations for error states when the label is inline, it makes sense in my opinion to make those accommodations for the hint text too. What do we think @edleeks87 @ljemmo ?

font-size: ${isLarge
? "var(--global-font-static-body-regular-l)"
: "var(--global-font-static-body-regular-m)"};
font-style: normal;
font-weight: ${isError ? "500" : "400"};
line-height: 150%;
Comment on lines +32 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

question: would these need to be tokens, too? Or are we leaving those aside for now?

Copy link
Contributor

Choose a reason for hiding this comment

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

question: would these need to be tokens, too? Or are we leaving those aside for now?

@damienrobson-sage we don't currently distribute core tokens like line-height or font-weight. instead Font tokens can be applied which contain those core values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this is due to a little misunderstanding on my part, some hard coded font-familys, weights, colours, sizes etc. have been used instead of the font shorthand with the corresponding font token. I'll make sure this is addressed where i've used them 👍

`;
}}
`;

export default StyledValidationMessage;
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import React from "react";
import { render, screen } from "@testing-library/react";
import ValidationMessage from ".";

test("should not render when neither error nor warning is provided", () => {
render(<ValidationMessage data-role="validation-message" />);
expect(screen.queryByTestId("validation-message")).not.toBeInTheDocument();
});

test("should render when error is a string", () => {
render(<ValidationMessage error="Error message" />);
const validationMessage = screen.getByText("Error message");
expect(validationMessage).toBeVisible();
});

test("should render when warning is a string", () => {
render(<ValidationMessage warning="Warning message" />);
const validationMessage = screen.getByText("Warning message");
expect(validationMessage).toBeVisible();
});

test("should render with id attribute", () => {
render(<ValidationMessage id="validation-error-1" error="Error message" />);
const validationMessage = screen.getByText("Error message");
expect(validationMessage).toHaveAttribute("id", "validation-error-1");
});

test("should support custom data-role attribute", () => {
render(
<ValidationMessage error="Error occurred" data-role="custom-validation" />,
);
const validationMessage = screen.getByTestId("custom-validation");
expect(validationMessage).toBeVisible();
});

test("should support data-element attribute", () => {
render(
<ValidationMessage error="Error message" data-element="validation-error" />,
);
const validationMessage = screen.getByText("Error message");
expect(validationMessage).toHaveAttribute("data-element", "validation-error");
});

test("should apply error color when error is provided", () => {
render(<ValidationMessage error="Error message" />);
const validationMessage = screen.getByText("Error message");

expect(validationMessage).toHaveStyle(
"color: var(--input-validation-label-error)",
);
});

test("should apply error font-weight", () => {
render(<ValidationMessage error="Error message" />);
const validationMessage = screen.getByText("Error message");
expect(validationMessage).toHaveStyle("font-weight: 500");
});

test("should apply warning color when warning is provided", () => {
render(<ValidationMessage warning="Warning message" />);
const validationMessage = screen.getByText("Warning message");
expect(validationMessage).toHaveStyle(
"color: var(--input-validation-label-warn)",
);
});

test("should apply warning font-weight", () => {
render(<ValidationMessage warning="Warning message" />);
const validationMessage = screen.getByText("Warning message");
expect(validationMessage).toHaveStyle("font-weight: 400");
});

test("should apply medium font size by default for error", () => {
render(<ValidationMessage error="Error message" />);
const validationMessage = screen.getByText("Error message");
expect(validationMessage).toHaveStyle(
"font-size: var(--global-font-static-body-regular-m)",
);
});

test("should apply large font size when isLarge is true for error", () => {
render(<ValidationMessage error="Error message" isLarge={true} />);
const validationMessage = screen.getByText("Error message");
expect(validationMessage).toHaveStyle(
"font-size: var(--global-font-static-body-regular-l)",
);
});

test("should apply medium font size by default for warning", () => {
render(<ValidationMessage warning="Warning message" />);
const validationMessage = screen.getByText("Warning message");
expect(validationMessage).toHaveStyle(
"font-size: var(--global-font-static-body-regular-m)",
);
});

test("should apply large font size when isLarge is true for warning", () => {
render(<ValidationMessage warning="Warning message" isLarge={true} />);
const validationMessage = screen.getByText("Warning message");
expect(validationMessage).toHaveStyle(
"font-size: var(--global-font-static-body-regular-l)",
);
});

test("should prioritize error over warning when both provided", () => {
render(<ValidationMessage error="Error message" warning="Warning message" />);
expect(screen.getByText("Error message")).toBeVisible();
expect(screen.queryByText("Warning message")).not.toBeInTheDocument();
});

test("should apply error styling when both error and warning strings are provided", () => {
render(<ValidationMessage error="Error message" warning="Warning message" />);
const validationMessage = screen.getByText("Error message");
expect(validationMessage).toHaveStyle(
"color: var(--input-validation-label-error)",
);
expect(validationMessage).toHaveStyle("font-weight: 500");
});
Copy link
Contributor

Choose a reason for hiding this comment

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

comment (non-blocking): if this is the same for all components I wonder if it's more appropriate in the shared _internal_ directory (still as a _next_)

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import styled, { css } from "styled-components";

const ErrorBorder = styled.span`
${({ warning }: { warning: boolean }) => css`
width: 2px;
position: absolute;
left: -10px;
top: 0px;
bottom: 0px;
z-index: 6;
background-color: ${warning
? "var(--input-validation-bar-warn)"
: "var(--input-validation-border-error)"};
`}
transform: scaleX(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

question: is this transform doing anything here as it's scaling to 100% with no animation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I added this as when the window is being repositioned the absolutely positioned error border can sometimes shrink when the label is inline. We didn't observe this previously as the error border was always in a fixed position, however when inline it is in the centre of the screen and needs to reposition. Adding the transform stopped this shrinking from occurring, and it appeared to be the only solution I could find that worked.

`;

export default ErrorBorder;
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import React from "react";
import { render, screen } from "@testing-library/react";
import ErrorBorder from "./error-border.style";

test("when `warning` is true should render with warning background colour", () => {
render(<ErrorBorder data-role="error-border" warning />);

expect(screen.getByTestId("error-border")).toHaveStyleRule(
"background-color",
"var(--input-validation-bar-warn)",
);
});

test("when `warning` is false should render with error background colour", () => {
render(<ErrorBorder data-role="error-border" warning={false} />);

expect(screen.getByTestId("error-border")).toHaveStyleRule(
"background-color",
"var(--input-validation-border-error)",
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React from "react";
import StyledHintText from "./hint-text.style";

export interface HintTextProps {
/** Children elements */
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: Child elements or Children 😆

children?: React.ReactNode;
/** Sets the id for the component */
id?: string;
/** If true, uses large font size */
isLarge?: boolean;
/** If true, the hint text will display in disabled styling */
disabled?: boolean;
}

export const HintText = ({
children,
id,
isLarge,
disabled,
}: HintTextProps) => {
return (
<StyledHintText
data-element="input-hint"
data-role="hint-text"
id={id}
isLarge={isLarge}
disabled={disabled}
>
{children}
</StyledHintText>
);
};

export default HintText;
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import styled from "styled-components";

interface StyledHintTextProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion:

interface StyledHintTextProps {
  $isLarge?: boolean;
  $disabled?: boolean;
}

isLarge?: boolean;
disabled?: boolean;
}

const StyledHintText = styled.span<StyledHintTextProps>`
color: ${({ disabled }) =>
disabled
? "var(--input-labelset-label-disabled)"
: "var(--input-labelset-label-alt)"};
font-family: var(--fontFamiliesDefault);
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment: same as above wrt to using font and the token etc

font-size: ${({ isLarge }) =>
isLarge
? "var(--global-font-static-body-regular-l)"
: "var(--global-font-static-body-regular-m)"};
font-style: normal;
font-weight: 400;
line-height: 150%;
`;

export default StyledHintText;
Loading
Loading