-
Notifications
You must be signed in to change notification settings - Fork 64
Fixing enabled and visible attributes of Button CC #1308
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
Conversation
Lighthouse scores (desktop)
|
Lighthouse scores (mobile)
|
Accessibility Violations Found
|
2 similar comments
Accessibility Violations Found
|
Accessibility Violations Found
|
Accessibility Violations Found
|
1 similar comment
Accessibility Violations Found
|
Lighthouse scores (mobile)
|
Lighthouse scores (desktop)
|
Accessibility Violations Found
|
e49ad42 to
de5dfb6
Compare
1130fb6 to
370d255
Compare
Lighthouse scores (desktop)
|
Lighthouse scores (mobile)
|
Accessibility Violations Found
|
2 similar comments
Accessibility Violations Found
|
Accessibility Violations Found
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1308 +/- ##
=========================================
Coverage 81.73% 81.73%
Complexity 853 853
=========================================
Files 97 97
Lines 2267 2267
Branches 305 305
=========================================
Hits 1853 1853
Misses 255 255
Partials 159 159 ☔ View full report in Codecov by Sentry. |
| expect(formContainer._model.items.length, "model and view elements match").to.equal(Object.keys(formContainer._fields).length); | ||
| Object.entries(formContainer._fields).forEach(([id, field]) => { | ||
| if(field.options.visible === "true") { | ||
| if(field.options.visible === "true" && field.getModel()._jsonModel.fieldType==='text-input') { |
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.
Why is there a change in an existing test case ? This can impact backward compatibility
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.
Attributes 'data-cmp-enabled' and 'data-cmp-visible' are causing button to be present always in the form which is why the checkHTML will fail if we do not explicitly check for 'text-input'. I have confirmed all components in both textinput and customtextinput test files have all components with fieldType 'text-input' apart from a submit button.
rismehta
left a comment
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.
check comments
* Fixing enabled and visible attributes of Button CC * Updating test cases to check fieldType to support button attribute changes --------- Co-authored-by: Muskan Gupta <muskgupta@Muskans-MacBook-Pro.local>
Description
Fixed the enabled and visible attributes to reflect correctly in forms for button core component.
JIRA : https://jira.corp.adobe.com/browse/FORMS-15167
Related Issue
Motivation and Context
Without this change, the button component in forms was not correctly populating true and false values to 'data-cmp-enabled' and 'data-cmp-visible' attributes.
How Has This Been Tested?
Tested the changes using cypress.
Screenshots (if appropriate):
Types of changes
Checklist: