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

feat(TextareaField): support recommendedMaxLength prop for display-only errors #1769

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

booc0mtaco
Copy link
Contributor

@booc0mtaco booc0mtaco commented Sep 29, 2023

Summary:

  • new prop recommendedLength allows for showing an error but letting the user continue typing content
  • maxLength still works, and can be a different (larger or equal) value
  • update snapshots and tests

Test Plan:

  • Wrote automated tests
  • CI tests / new tests are not applicable
  • Manually tested my changes, but I want to keep the details secret
  • Manually tested my changes, and here are the details:
    • Create an alpha publish and try out in edu-stack or traject as a sanity check if changes affect build or deploy, or are breaking, such as token changes, widely used component updates, hooks changes, and major dependency upgrades.

@booc0mtaco
Copy link
Contributor Author

booc0mtaco commented Sep 29, 2023

TBD:

  • unit tests to pressure test the input patterns
  • verify when to show/hide the field note

@github-actions
Copy link

github-actions bot commented Sep 29, 2023

size-limit report 📦

Path Size
components 96.49 KB (+0.25% 🔺)
styles 32.79 KB (0%)

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #1769 (9e43020) into next (f231ad4) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 9e43020 differs from pull request most recent head 9dc9bf2. Consider uploading reports for the commit 9dc9bf2 to get more accurate results

@@            Coverage Diff             @@
##             next    #1769      +/-   ##
==========================================
- Coverage   92.29%   92.29%   -0.01%     
==========================================
  Files         146      146              
  Lines        2623     2635      +12     
  Branches      690      697       +7     
==========================================
+ Hits         2421     2432      +11     
  Misses        187      187              
- Partials       15       16       +1     
Files Coverage Δ
src/components/TextareaField/TextareaField.tsx 97.18% <100.00%> (+0.57%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@booc0mtaco
Copy link
Contributor Author

Screen.Recording.2023-09-29.at.18.14.37.mov

…ly errors

- new prop allows for showing an error but letting the user continue
  typing content
- maxLength still works, and can be a different (larger or equal) value
- update snapshots
- add tests
@booc0mtaco booc0mtaco marked this pull request as ready for review October 2, 2023 16:26
@booc0mtaco booc0mtaco requested a review from a team October 2, 2023 16:26
Comment on lines +124 to +132
function getSmallest(lengthA?: number, lengthB?: number): number | undefined {
if (lengthA !== undefined && lengthB !== undefined) {
return Math.min(lengthA, lengthB);
} else if (lengthA && lengthB === undefined) {
return lengthA;
} else if (lengthB && lengthA === undefined) {
return lengthB;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Code golf time 🏌️‍♀️

Suggested change
function getSmallest(lengthA?: number, lengthB?: number): number | undefined {
if (lengthA !== undefined && lengthB !== undefined) {
return Math.min(lengthA, lengthB);
} else if (lengthA && lengthB === undefined) {
return lengthA;
} else if (lengthB && lengthA === undefined) {
return lengthB;
}
}
function getSmallest(...args: Array<number | undefined>): number | undefined {
if (args.length === 0) {
return;
}
return Math.min(...args.filter(Boolean) as number[]);
}

or with type predicate on the filter:

Suggested change
function getSmallest(lengthA?: number, lengthB?: number): number | undefined {
if (lengthA !== undefined && lengthB !== undefined) {
return Math.min(lengthA, lengthB);
} else if (lengthA && lengthB === undefined) {
return lengthA;
} else if (lengthB && lengthA === undefined) {
return lengthB;
}
}
function getSmallest(...args: Array<number | undefined>): number | undefined {
if (args.length === 0) {
return;
}
return Math.min(...args.filter((n): n is number => Boolean(n)));
}

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 like these ⛳! Since the other PR builds upon this one and moves this into a utility, will commit this as-is, and take some strokes off in #1771

@booc0mtaco booc0mtaco merged commit 0852356 into next Oct 3, 2023
7 checks passed
@booc0mtaco booc0mtaco deleted the aholloway/EFI-1382 branch October 3, 2023 02:31
@booc0mtaco booc0mtaco mentioned this pull request Oct 3, 2023
booc0mtaco added a commit that referenced this pull request Oct 4, 2023
## [13.4.0](v13.3.0...v13.4.0) (2023-10-03)


### Features

* **Accordion:** allow empty or hidden accordion rows ([#1767](#1767)) ([e044a85](e044a85))
* **Icons:** allow component icon usages to be headless ([#1761](#1761)) ([ba454bf](ba454bf))
* **InputField:** support recommendedMaxLength prop for display-only errors ([#1771](#1771)) ([cc84a20](cc84a20))
* **Tabs:** add ability to customize tab button headers ([#1768](#1768)) ([f231ad4](f231ad4))
* **TextareaField:** support recommendedMaxLength prop for display-only errors ([#1769](#1769)) ([0852356](0852356))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants