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.

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

Jump to bottom

feat(rating, slider): add validation message to support form error handling #10621

Merged
merged 8 commits into from
Oct 29, 2024
Merged

feat(rating, slider): add validation message to support form error handling #10621

merged 8 commits into from
Oct 29, 2024

Conversation

aPreciado88
Copy link
Contributor

@aPreciado88 aPreciado88 commented Oct 25, 2024 •
edited
Loading

Related Issue: #9999

Summary

Add validationMessage, validationIcon and status props to rating and slider components to support form validation.

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Oct 25, 2024
@aPreciado88 aPreciado88 added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Oct 25, 2024
@aPreciado88
Copy link
Contributor Author

@ashetland @SkyeSeitz Can you please take a look at the chromatic build?

benelan
benelan reviewed Oct 25, 2024
View reviewed changes
Copy link
Member

@benelan benelan left a comment

Choose a reason for hiding this comment

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

Looking good! It looks like some code marked as "do not edit" was edited, can you take another look at that part?

packages/calcite-components/.stylelintrc.cjs Outdated
@@ -1,7 +1,13 @@
// @ts-check

// ⚠️ AUTO-GENERATED CODE - DO NOT EDIT
const customFunctions = [];
const customFunctions = [
"get-trailing-text-input-padding",
Copy link
Member

Choose a reason for hiding this comment

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

This code is auto-generated by this script and shouldn't be edited directly. IIRC @jcfranco set it up if you need more info.

benelan
benelan approved these changes Oct 25, 2024
View reviewed changes
Copy link
Member

@benelan benelan left a comment

Choose a reason for hiding this comment

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

👍

benelan
benelan approved these changes Oct 25, 2024
View reviewed changes
Copy link
Member

@benelan benelan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Edit: Woops, double approved because it said the first one didn't go through lol. The code changes besides stylelintrc LGTM

@aPreciado88 aPreciado88 added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 25, 2024
@aPreciado88
Copy link
Contributor Author

@ashetland I pulled dev and ran a new chromatic build. Could you please take another look?

@ashetland
Copy link
Contributor

@ashetland I pulled dev and ran a new chromatic build. Could you please take another look?

That fixed it. Just the one remaining conversation to resolve.

@aPreciado88
feat(rating, slider): remove validation message top padding in slider
10a4bd0
@aPreciado88
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
9ebb828 
…ciado88/9999-add-form-support-via-required-and-validationmessage-for-error-handling
@aPreciado88
Merge branch 'aPreciado88/9999-add-form-support-via-required-and-vali…
7d5b964 
…dationmessage-for-error-handling' of github.com:Esri/calcite-design-system into aPreciado88/9999-add-form-support-via-required-and-validationmessage-for-error-handling
@aPreciado88 aPreciado88 added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 28, 2024
@aPreciado88
Copy link
Contributor Author

@ashetland I updated the error message's top padding within the slider component. Can you please take a look?

@aPreciado88 aPreciado88 added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 28, 2024
benelan
benelan requested changes Oct 28, 2024
View reviewed changes
Copy link
Member

@benelan benelan left a comment

Choose a reason for hiding this comment

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

Requesting changes until the padding discrepancies are resolved so this doesn't get accidentally merged (I've been there)

@ashetland
Copy link
Contributor

Visuals lookin' good!

benelan
benelan approved these changes Oct 28, 2024
View reviewed changes
Copy link
Member

@benelan benelan left a comment

Choose a reason for hiding this comment

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

LGTM!

@aPreciado88 aPreciado88 merged commit c4c9b1d into dev Oct 29, 2024
19 checks passed
@aPreciado88 aPreciado88 deleted the aPreciado88/9999-add-form-support-via-required-and-validationmessage-for-error-handling branch October 29, 2024 00:14
benelan pushed a commit that referenced this pull request Feb 8, 2025
@aPreciado88 @benelan
feat(rating, slider): add validation message to support form error ha…
b632449 
…ndling (#10621)

**Related Issue:**
[#9999](#9999)

## Summary

Add `validationMessage`, `validationIcon` and `status` props to `rating`
and `slider` components to support form validation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashetland ashetland ashetland left review comments

@benelan benelan benelan approved these changes

@jcfranco jcfranco Awaiting requested review from jcfranco jcfranco is a code owner

Assignees

@aPreciado88 aPreciado88

Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Milestone
2024-10-29 - Oct Milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aPreciado88 @ashetland @benelan