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(forms): add required mark class #606

Merged
merged 6 commits into from
Apr 20, 2023
Merged

feat(forms): add required mark class #606

merged 6 commits into from
Apr 20, 2023

Conversation

dtsanevmw
Copy link
Contributor

@dtsanevmw dtsanevmw commented Mar 28, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • If applicable, have a visual design approval

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

All fields are assumed required and there is no way to tell that.

Issue Number: CDE-16, #623

What is the new behavior?

Added a class which can be used to mark a container as required.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@github-actions
Copy link
Contributor

github-actions bot commented Mar 28, 2023

👋 @dtsanevmw,

  • 🙏 The Clarity team thanks you for opening a pull request
  • 🎉 The build for this PR has succeeded
  • 🔍 The PR is now ready for review
  • 🍿 In the meantime, checkout out a preview of this PR
  • 🖐 You can always follow up here. If you're a VMware employee, you can also reach us on our internal #clarity-support Slack channel

Thank you,

🤖 Clarity Release Bot

@github-actions
Copy link
Contributor

This PR introduces visual changes: 2f8c6fd
If these changes are intended and correct, please cherry-pick the above commit to this PR.

git checkout dtsanevmw/cde-16
git fetch https://github.com/vmware-clarity/ng-clarity.git 2f8c6fd492db978fdae484dcd8d69f1854b241a2
git cherry-pick 2f8c6fd492db978fdae484dcd8d69f1854b241a2
git push

@kevinbuhmann kevinbuhmann changed the title fix(forms): required mark class feat(forms): add required mark class Mar 28, 2023
kevinbuhmann
kevinbuhmann previously approved these changes Mar 28, 2023
@kevinbuhmann kevinbuhmann self-requested a review March 30, 2023 20:47
@kevinbuhmann kevinbuhmann dismissed their stale review March 30, 2023 20:50

I'm no longer sure if a CSS class the best way to do this. One thing we need to determine is how this shoukd work for screen reader users. This might turn out to be a bit more involved.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

This PR introduces visual changes: 5da5302
If these changes are intended and correct, please cherry-pick the above commit to this PR.

git checkout dtsanevmw/cde-16
git fetch https://github.com/vmware-clarity/ng-clarity.git 5da530270adf7629a06f064fab014c4e3131a52c
git cherry-pick 5da530270adf7629a06f064fab014c4e3131a52c
git push

Copy link
Member

@kevinbuhmann kevinbuhmann left a comment

Choose a reason for hiding this comment

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

We need a11y review before merging.

Copy link

@AmyLiNow AmyLiNow left a comment

Choose a reason for hiding this comment

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

verified that asterisks are read with VO/Safari and NVDA/chrome.

@dtsanevmw dtsanevmw merged commit deaf281 into main Apr 20, 2023
3 checks passed
@dtsanevmw dtsanevmw deleted the dtsanevmw/cde-16 branch April 20, 2023 07:42
dtsanevmw added a commit that referenced this pull request Apr 20, 2023
## PR Checklist

Please check if your PR fulfills the following requirements:

- [x] Tests for the changes have been added (for bug fixes / features)
- [x] Docs have been added / updated (for bug fixes / features)
- [x] If applicable, have a visual design approval

## PR Type

What kind of change does this PR introduce?

<!-- Please check the one that applies to this PR using "x". -->

- [ ] Bugfix
- [x] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Documentation content changes
- [ ] Other... Please describe:

## What is the current behavior?

All fields are assumed required and there is no way to tell that.

Issue Number: CDE-16, #623

## What is the new behavior?

Added a class which can be used to mark a container as required.

## Does this PR introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this PR contains a breaking change, please describe the impact
and migration path for existing applications below. -->

## Other information

---------

Co-authored-by: GitHub <noreply@github.com>
dtsanevmw added a commit that referenced this pull request Apr 20, 2023
backport of #606

---------

Co-authored-by: GitHub <noreply@github.com>
@github-actions
Copy link
Contributor

🎉 This PR is included in version 15.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed PRs after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.

@github-actions github-actions bot locked and limited conversation to collaborators May 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants