-
Notifications
You must be signed in to change notification settings - Fork 156
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
Combine Email and Phone into single choice on forms #12127
Combine Email and Phone into single choice on forms #12127
Conversation
✅ Deploy Preview for ibm-dotcom-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I have read the DCO document and I hereby sign the DCO. |
✅ Deploy Preview for ibm-dotcom-web-components-react-wrap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -56,6 +56,9 @@ class NoticeChoice extends StableSelectorMixin(LitElement) { | |||
@property({ type: String, attribute: 'language' }) | |||
language = 'en'; | |||
|
|||
@property({ type: String, attribute: 'currentLanguage' }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@property({ type: String, attribute: 'currentLanguage' }) | |
@property({ type: String, attribute: 'current-language' }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not yet resolved. Attribute names are snake case by convention, not camel case. Any particular reason to go against the convention for this one out of all the others?
241ac04
to
32c6e01
Compare
Deploy preview created for package Built with commit: 23edf1a16259d74d7381d900f4330aa32331cc65 |
2d90c3a
to
df8f757
Compare
df8f757
to
d489801
Compare
Deploy preview created for package Built with commit: 23edf1a16259d74d7381d900f4330aa32331cc65 |
d23c81e
to
5219f88
Compare
caaeca9
to
5fbaf61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good except one small nit pick.
@@ -56,6 +56,9 @@ class NoticeChoice extends StableSelectorMixin(LitElement) { | |||
@property({ type: String, attribute: 'language' }) | |||
language = 'en'; | |||
|
|||
@property({ type: String, attribute: 'currentLanguage' }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not yet resolved. Attribute names are snake case by convention, not camel case. Any particular reason to go against the convention for this one out of all the others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second though, lets push this through for now and address the nit in a follow up. We're stretching to release an RC today if we can.
Follow up PR: #12147 |
### Related Ticket(s) [Noitce & Choice w3 Publisher](https://w3.ibm.com/w3publisher/urx/notice-and-choice-v23) ### Description Notice & Choice is a legally required component to create a form where IBM collects its customer information. This section shows the specific product's legal links, terms, and conditions. Along with this, it collects users' consent regarding communication preferences. ### Changelog **New** - Combine Email and Phone into a single choice on forms **Changed** - Instead of pre-text followed by Email and Phone boxes, combine them into a single checkbox question. Text planned for this is: "I'd like IBM to use my contact details to keep me informed about products, services and offers. More information on how IBM uses data and ways to opt-out can be found in the IBM Privacy Statement.". "Opt-out" and "IBM Privacy Statement would be linked as usual. **Removed** <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "test: e2e": Codesandbox examples and e2e integration tests --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
### Related Ticket(s) Follow up to #12127 ### Description Following convention, switch `current-language` attribute to snake case. ### Changelog **Changed** - Changed `currentLanguage` attribute in `<c4d-notice-choice>` component to snake case <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "test: e2e": Codesandbox examples and e2e integration tests --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
Related Ticket(s)
Noitce & Choice w3 Publisher
Description
Notice & Choice is a legally required component to create a form where IBM collects its customer information.
This section shows the specific product's legal links, terms, and conditions. Along with this, it collects users' consent regarding communication preferences.
Changelog
New
Changed
Text planned for this is: "I'd like IBM to use my contact details to keep me informed about products, services and offers. More information on how IBM uses data and ways to opt-out can be found in the IBM Privacy Statement.". "Opt-out" and "IBM Privacy Statement would be linked as usual.
Removed