Skip to content
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

Add TextArea component #400

Merged
merged 47 commits into from
Jul 22, 2024
Merged

Add TextArea component #400

merged 47 commits into from
Jul 22, 2024

Conversation

mkernohanbc
Copy link
Contributor

This PR expands on #389 to add a new Text Area component.

Text Area is an alternative implementation of Text Field, designed for multi-line plain text input.

Changes from Text Field:

  • replaces the in the Text Field component with a <TextArea> (as recommended by RAC)
  • removes the size and type variables
  • removes the iconLeft and iconRight slots

Leaving this in draft for now, pending input from Philip on whether there should be any other design or behavioural variances between this and the base Text Field component.

@mkernohanbc mkernohanbc added the enhancement New feature or request label Jun 28, 2024
@mkernohanbc mkernohanbc self-assigned this Jun 28, 2024
@mkernohanbc mkernohanbc linked an issue Jun 28, 2024 that may be closed by this pull request
@mkernohanbc mkernohanbc requested a review from ty2k June 28, 2024 17:54
@mkernohanbc mkernohanbc changed the title Feature/textarea Add TextArea component Jun 28, 2024
@mkernohanbc
Copy link
Contributor Author

mkernohanbc commented Jul 3, 2024

@ty2k documenting our conversation for reference:

  • We have a case in the design spec for this component which displays a character counter.
  • I've created a secondary <span> that will render and display the value of maxLength when that prop is present:
Screenshot 2024-07-03 at 15 34 33
  • I'm not sure what the best method is to capture and display the character count of the input value
  • There doesn't seem to be a boilerplate solution to this within the RAC TextField or useTextField APIs

@mkernohanbc
Copy link
Contributor Author

mkernohanbc commented Jul 9, 2024

Flagging that the change in c9c44b5 is throwing a TS error in the console, but doesn't actually seem to affect how the component renders in Storybook:

Type 'string | ((validation: ValidationResult) => string) | undefined' is not assignable to type 'ReactNode'.
Type '(validation: ValidationResult) => string' is not assignable to type 'ReactNode'.

Needs further investigation.

@mkernohanbc
Copy link
Contributor Author

Flagging that the change in c9c44b5 is throwing a TS error in the console, but doesn't actually seem to affect how the component renders in Storybook:

Type 'string | ((validation: ValidationResult) => string) | undefined' is not assignable to type 'ReactNode'.
Type '(validation: ValidationResult) => string' is not assignable to type 'ReactNode'.

Needs further investigation.

This is fixed in a88ddc3. The Typescript error seemed to originate with the way we were implementing the warning icon within the FieldError. To resolve this:

  • removed iconError const from TextArea.tsx
  • moved warning icon SVG into /src/assets/icon-error.svg
  • use ::before pseudo-class in TextArea.css to display the icon alongside the error message

@mkernohanbc
Copy link
Contributor Author

mkernohanbc commented Jul 15, 2024

I've done some structural cleanup on this component over the past few days:

  • 5138089 and a56f44d: puts description and character counter into separate divs and adds styling rules so that they are properly positioned when one or both are present
  • 9d6a4b4: adds some conditional logic so that if neither description nor character counter are present, the component doesn't render an empty <div>

Only outstanding piece of work before this PR is ready for review is implementing the state handling for the character counter (awaiting @ty2k's input.)

Copy link
Contributor

@ty2k ty2k left a comment

Choose a reason for hiding this comment

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

This rocks @mkernohanbc . Let me know if you want to pair on the SVG icon issue. No need to replace the other instances of that icon in this PR (in Select and TextField), but let's add the common version here to start.

@mkernohanbc mkernohanbc requested a review from ty2k July 22, 2024 18:34
@mkernohanbc
Copy link
Contributor Author

@ty2k this should now be good to go!

Flagging that Storybook build issues I was experiencing this morning appeared to be fixed by bumping Storybook to 8.2.5 (see #411), but I've reverted those changes within this PR.

Copy link
Contributor

@ty2k ty2k left a comment

Choose a reason for hiding this comment

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

Awesome! We just need to clean up the SVG id stuff and this can be merged. 👍 👍

@mkernohanbc mkernohanbc requested a review from ty2k July 22, 2024 20:27
@mkernohanbc mkernohanbc requested a review from ty2k July 22, 2024 20:33
Copy link
Contributor

@ty2k ty2k left a comment

Choose a reason for hiding this comment

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

Looks great!

@mkernohanbc mkernohanbc merged commit 847942e into main Jul 22, 2024
4 checks passed
@mkernohanbc mkernohanbc deleted the feature/textarea branch July 24, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text Area
2 participants