-
Notifications
You must be signed in to change notification settings - Fork 38
add "for" accessibility to style editor elements, add form descriptio… #2011
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
…n font size option, add extra check for base font size in order to avoid undefined when used as non-numeric
WalkthroughThe changes involve modifications to various PHP files related to form styling and accessibility improvements. Key updates include adding Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (9)
classes/views/styles/_form-title.php (1)
42-46
: LGTM: Accessibility improvement for Margin label with a suggestionThe addition of the
for="frm_title_margins"
attribute to the label element is a positive change. It correctly associates the label with its corresponding input (slider component), enhancing accessibility for users relying on assistive technologies.However, given that the margin input consists of two independent fields (top and bottom), consider the following suggestion:
To further improve accessibility, you might want to add separate labels for the top and bottom margin inputs. This would allow screen readers to distinguish between the two inputs more clearly. Here's a potential implementation:
<div class="frm5 frm_form_field"> <label for="frm_title_margins" class="frm-style-item-heading"><?php esc_html_e( 'Margin', 'formidable' ); ?></label> </div> <div class="frm7 frm_form_field"> <?php new FrmSliderStyleComponent( null, $style->post_content['title_margin_top'], array( 'id' => 'frm_title_margins', 'type' => 'vertical-margin', 'max_value' => 100, 'independent_fields' => array( array( 'name' => $frm_style->get_field_name( 'title_margin_top' ), 'value' => $style->post_content['title_margin_top'], 'id' => 'frm_title_margin_top', 'type' => 'top', + 'label' => '<span class="frm_screen_reader_text">' . esc_html__( 'Top Margin', 'formidable' ) . '</span>', ), array( 'name' => $frm_style->get_field_name( 'title_margin_bottom' ), 'value' => $style->post_content['title_margin_bottom'], 'id' => 'frm_title_margin_bottom', 'type' => 'bottom', + 'label' => '<span class="frm_screen_reader_text">' . esc_html__( 'Bottom Margin', 'formidable' ) . '</span>', ), ), ) ); ?> </div>This change adds visually hidden labels for screen readers, providing more context for each margin input while maintaining the current visual design.
classes/views/styles/_form-description.php (1)
7-24
: LGTM! Consider adding a unit to the label for consistency.The new font size configuration section is well-structured and consistent with other sections. The use of
FrmSliderStyleComponent
and the inclusion of thefor
attribute in the label improve functionality and accessibility.For consistency with other size-related labels in the form (if any), consider adding the unit (e.g., "px" or "em") to the label:
- class="frm-style-item-heading"><?php esc_html_e( 'Font Size', 'formidable' ); ?> + class="frm-style-item-heading"><?php esc_html_e( 'Font Size (px)', 'formidable' ); ?>This change would provide users with immediate clarity about the unit of measurement being used.
classes/views/styles/_check-box-radio-fields.php (2)
61-63
: Great accessibility enhancement, with a minor suggestion.The addition of the
for="frm_check_align"
attribute to the label element is an excellent improvement for accessibility. This change ensures that the label is correctly associated with its corresponding dropdown input, enhancing the form's usability for users of assistive technologies.However, the label text "Check Box" might be slightly confusing as it refers to the alignment of checkboxes, not a single checkbox.
Consider changing the label text to "Checkbox Alignment" or "Checkbox Layout" for better clarity:
- class="frm-style-item-heading"><?php esc_html_e( 'Check Box', 'formidable' ); ?></label> + class="frm-style-item-heading"><?php esc_html_e( 'Checkbox Alignment', 'formidable' ); ?></label>
82-84
: Excellent accessibility improvement, with a minor suggestion.The addition of the
for="frm_radio_align"
attribute to the label element is a great enhancement for accessibility. This change ensures that the label is properly associated with its corresponding dropdown input, improving the form's usability for users of assistive technologies.However, similar to the previous suggestion, the label text "Radio" might be slightly confusing as it refers to the alignment of radio buttons, not a single radio button.
Consider changing the label text to "Radio Button Alignment" or "Radio Button Layout" for better clarity and consistency with the previous suggestion:
- class="frm-style-item-heading"><?php esc_html_e( 'Radio', 'formidable' ); ?></label> + class="frm-style-item-heading"><?php esc_html_e( 'Radio Button Alignment', 'formidable' ); ?></label>classes/views/styles/_field-description.php (1)
Mismatch Found in Multiple Sections
The verification script identified mismatches between
for
attributes in labels and their correspondingid
attributes in the following sections:
- Color
for
:frm_description_color
id
: (incorrectly matched)- Margin
for
:frm_description_margin
id
: (incorrectly matched)- Style
for
:frm_description_style
id
: (incorrectly matched)- Weight
for
:frm_description_weight
id
: (incorrectly matched)Action Required:
- Ensure that each
label
'sfor
attribute exactly matches theid
of its corresponding form control in all four sections listed above.Once these mismatches are corrected, the accessibility improvements will be fully realized across all form styling controls.
🔗 Analysis chain
Line range hint
1-118
: Summary of accessibility improvements and remaining issueOverall, the changes in this file significantly improve accessibility by adding
for
attributes to label elements and ensuring they match theid
attributes of their corresponding form controls. This aligns well with the PR objectives of enhancing accessibility features.However, there is one issue that needs to be addressed:
- In the Font Size section (lines 25-27), the
for
attribute of the label and theid
of the slider component are mismatched. This should be corrected to ensure proper accessibility for the Font Size control.Once this issue is resolved, the accessibility improvements in this file will be complete and consistent across all form styling controls.
To ensure all
for
attributes match their correspondingid
attributes, you can run the following script:This script will help identify any remaining mismatches between
for
andid
attributes in the file.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all 'for' attributes match their corresponding 'id' attributes # Test: Check for mismatches between 'for' and 'id' attributes rg --type php -e 'for="([^"]+)"' -e 'id="([^"]+)"' classes/views/styles/_field-description.php | \ awk -F'"' '{print $2}' | sort | uniq -c | awk '$1 == 1 {print "Mismatch found for: " $2}'Length of output: 358
classes/views/styles/_field-labels.php (1)
57-59
: LGTM with a suggestion: Improved accessibility for the Weight labelThe addition of the
for
attribute to the label element enhances accessibility by associating it with the corresponding input field (id="frm_required_weight"
). This change improves the user experience for those using assistive technologies.However, there's a minor inconsistency between the label text "Weight" and the input ID "frm_required_weight". Consider clarifying whether this is for general weight or specifically for required fields.
Consider updating either the label text or the input ID for consistency:
- If it's for required fields: Change the label to "Required Weight"
- If it's for general weight: Change the input ID to "frm_weight"
classes/helpers/FrmStylesHelper.php (3)
Line range hint
4-4
: Consider addressing TODO comments.There are TODO comments in the file, such as the one on line 4. These indicate incomplete features or documentation. It's good practice to address these comments to ensure all features are fully implemented and documented.
Would you like me to create a GitHub issue to track the completion of these TODO items?
Line range hint
1068-1072
: Remove deprecated methods in future versions.The
maybe_include_font_icon_css
method is marked as deprecated. Consider removing this method in future versions to keep the codebase clean and prevent usage of outdated functionality.In the next major version update, remove this deprecated method and update any remaining calls to it throughout the codebase.
Line range hint
1-1072
: Consider refactoring for better separation of concerns.The
FrmStylesHelper
class has a large number of responsibilities, including color manipulation, CSS generation, and various utility functions. This might violate the Single Responsibility Principle.Consider refactoring this class into smaller, more focused classes. For example:
ColorManipulator
for color-related functionsCSSGenerator
for CSS and variable generationFontSizeCalculator
for font size related calculationsThis would improve maintainability and make the code easier to test and extend in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
- classes/helpers/FrmStylesHelper.php (1 hunks)
- classes/views/styles/_buttons.php (18 hunks)
- classes/views/styles/_check-box-radio-fields.php (5 hunks)
- classes/views/styles/_field-colors.php (16 hunks)
- classes/views/styles/_field-description.php (5 hunks)
- classes/views/styles/_field-labels.php (8 hunks)
- classes/views/styles/_field-sizes.php (7 hunks)
- classes/views/styles/_form-description.php (1 hunks)
- classes/views/styles/_form-messages.php (8 hunks)
- classes/views/styles/_form-title.php (3 hunks)
- classes/views/styles/_general.php (8 hunks)
- classes/views/styles/_quick-settings.php (10 hunks)
🔇 Additional comments (80)
classes/views/styles/_form-title.php (3)
6-10
: LGTM: Accessibility improvement for Color labelThe addition of the
for="frm_fieldset_color"
attribute to the label element is a positive change. It correctly associates the label with its corresponding input (color picker), enhancing accessibility for users relying on assistive technologies.
24-28
: LGTM: Accessibility improvement for Font Size labelThe addition of the
for="frm_title_size"
attribute to the label element is a positive change. It correctly associates the label with its corresponding input (slider component), enhancing accessibility for users relying on assistive technologies.
Line range hint
1-73
: Overall assessment: Accessibility improvements successfully implementedThe changes made to this file consistently improve accessibility by adding
for
attributes to all label elements, correctly associating them with their corresponding inputs. These modifications align well with the PR objectives of enhancing accessibility for the style editor elements.The code structure remains clean and consistent throughout the file. No additional issues or necessary improvements were identified beyond the suggestion for the margin inputs.
classes/views/styles/_check-box-radio-fields.php (4)
7-9
: Excellent accessibility improvement!The addition of the
for="frm_check_label_color"
attribute to the label element is a great enhancement for accessibility. This change ensures that the label is properly associated with its corresponding input field, making it easier for users of assistive technologies to interact with the form.
25-27
: Great accessibility enhancement!Adding the
for="frm_check_font_size"
attribute to the label element is an excellent improvement for accessibility. This change ensures that the label is correctly associated with its corresponding input field, enhancing the user experience for those using assistive technologies.
43-45
: Excellent accessibility improvement!The addition of the
for="frm_check_weight"
attribute to the label element is a valuable enhancement for accessibility. This change ensures that the label is properly associated with its corresponding dropdown input, improving the form's usability for users of assistive technologies.
Line range hint
1-101
: Overall excellent accessibility improvements!The changes made in this file consistently enhance the accessibility of the form by adding
for
attributes to all label elements. This improvement ensures that each label is properly associated with its corresponding input field, significantly benefiting users of assistive technologies.The implementation is consistent across all modified segments, demonstrating a thorough approach to improving the form's accessibility. These changes align perfectly with the PR objectives and contribute to a more inclusive user experience.
Great job on these accessibility enhancements!
classes/views/styles/_field-description.php (5)
7-9
: LGTM: Accessibility improvement for Color fieldThe addition of the
for
attribute to the label element, matching theid
of the color picker component, enhances accessibility. This change correctly associates the label with its form control, following best practices for web accessibility.
43-45
: LGTM: Accessibility improvement for Weight fieldThe addition of the
for
attribute to the label element, matching theid
of the dropdown component, enhances accessibility. This change correctly associates the label with its form control, following best practices for web accessibility.
61-63
: LGTM: Accessibility improvement for Style fieldThe changes in this section enhance accessibility:
- Addition of the
for
attribute to the label element.- Updating the
id
of the dropdown component to "frm_description_style".- Ensuring the
for
attribute matches the updatedid
.These modifications correctly associate the label with its form control, following best practices for web accessibility.
Also applies to: 71-71
82-84
: LGTM: Accessibility improvement for Align fieldThe changes in this section enhance accessibility:
- Addition of the
for
attribute to the label element.- Explicit setting of the
id
for the alignment component to "frm_description_align".- Ensuring the
for
attribute matches theid
of the component.These modifications correctly associate the label with its form control, following best practices for web accessibility.
Also applies to: 92-92
100-102
: LGTM: Accessibility improvement for Margin fieldThe addition of the
for
attribute to the label element, matching theid
of the slider component, enhances accessibility. This change correctly associates the label with its form control, following best practices for web accessibility.classes/views/styles/_field-sizes.php (7)
7-9
: LGTM: Accessibility improvement for Font Size fieldThe addition of the
for
attribute to the label, matching theid
of the corresponding input component, enhances accessibility. This change follows best practices for form accessibility.
25-27
: LGTM: Accessibility improvement for Weight fieldThe addition of the
for
attribute to the label, matching theid
of the corresponding input component, enhances accessibility. This change follows best practices for form accessibility.
43-45
: LGTM: Accessibility improvement for Height fieldThe addition of the
for
attribute to the label, matching theid
of the corresponding input component, enhances accessibility. This change follows best practices for form accessibility.
60-62
: LGTM: Accessibility improvement for Width fieldThe addition of the
for
attribute to the label, matching theid
of the corresponding input component, enhances accessibility. This change follows best practices for form accessibility.
77-79
: LGTM: Accessibility improvement for Padding fieldThe addition of the
for
attribute to the label, matching theid
of the corresponding input component, enhances accessibility. This change follows best practices for form accessibility.
95-97
: LGTM: Accessibility improvement for Margin fieldThe addition of the
for
attribute to the label, matching theid
of the corresponding input component, enhances accessibility. This change follows best practices for form accessibility.
113-115
: LGTM: Accessibility improvement for Corner Radius fieldThe addition of the
for
attribute to the label, matching theid
of the corresponding input component, enhances accessibility. This change follows best practices for form accessibility.Summary of changes:
All modified segments in this file consistently addfor
attributes to labels, improving accessibility for the following fields:
- Font Size
- Weight
- Height
- Width
- Padding
- Margin
- Corner Radius
These changes align with the PR objectives of enhancing accessibility features within the style editor elements.
classes/views/styles/_form-messages.php (7)
21-23
: LGTM: Improved accessibility for success background color labelThe addition of the
for
attribute to the label element, matching theid
of its corresponding input field, enhances accessibility. This change allows users of assistive technologies to better navigate and interact with the form.
39-41
: LGTM: Enhanced accessibility for success border color labelThe
for
attribute added to the label element, correctly associated with its input field'sid
, improves the form's accessibility. This change is consistent with best practices and helps users navigate the form more effectively.
57-59
: LGTM: Accessibility improvement for success font color labelThe addition of the
for
attribute to the label, correctly linked to its input field'sid
, enhances the form's accessibility. This change maintains consistency with previous improvements and adheres to accessibility best practices.
74-78
: LGTM: New success font size field with proper accessibilityA new form field for success message font size has been added, complete with an accessible label using the
for
attribute. This addition aligns with the PR objectives and maintains consistency with the existing code structure and accessibility improvements.
97-99
: LGTM: Improved accessibility for error background color labelThe addition of the
for
attribute to the label element, correctly associated with its input field'sid
, enhances the accessibility of the error message section. This change maintains consistency with previous improvements and adheres to accessibility best practices.
115-117
: LGTM: Comprehensive accessibility improvements for error message stylingThe changes in this segment consistently improve accessibility across the error message styling options:
- Error border color label (lines 115-117)
- Error font color label (lines 133-135)
- New error font size field and label (lines 150-154)
All labels now include
for
attributes correctly associated with their corresponding input fieldid
s. The addition of the error font size field maintains consistency with the success message section and aligns with the PR objectives.These changes demonstrate a thorough approach to enhancing form accessibility and usability.
Also applies to: 133-135, 150-154
Line range hint
1-170
: Summary: Excellent accessibility improvements and feature additionsThe changes in this file demonstrate a comprehensive approach to enhancing form accessibility and user customization options:
Accessibility improvements: All form labels now include
for
attributes correctly associated with their corresponding input fields, significantly improving the form's usability for users of assistive technologies.New font size options: Font size controls have been added for both success and error messages, allowing for greater customization of form appearance. This addition aligns well with the PR objectives.
Consistency: The changes are implemented consistently across both the success and error message sections, maintaining a cohesive user experience.
These modifications collectively contribute to a more accessible, user-friendly, and customizable form styling interface. Great work on improving the overall quality of the Formidable Forms plugin!
classes/views/styles/_field-labels.php (7)
21-23
: LGTM: Improved accessibility for the Color labelThe addition of the
for
attribute to the label element enhances accessibility by correctly associating it with the corresponding input field (id="frm_label_color"
). This change allows users of assistive technologies to better interact with the form.
39-41
: LGTM: Improved accessibility for the Font Size labelThe addition of the
for
attribute to the label element enhances accessibility by correctly associating it with the corresponding input field (id="frm_font_size"
). This change improves the user experience for those using assistive technologies.
75-77
: LGTM: Improved accessibility for the Position labelThe addition of the
for
attribute to the label element enhances accessibility by correctly associating it with the corresponding input field (id="frm_position"
). This change improves the user experience for those using assistive technologies.
93-95
: LGTM: Improved accessibility and consistency for the Align labelThe changes enhance accessibility and maintain consistency:
- The addition of the
for
attribute to the label element correctly associates it with the input field.- The input field's
id
has been updated to "frm_label_align" to match the label'sfor
attribute.These modifications improve the user experience for those using assistive technologies and ensure proper label-input association.
Also applies to: 103-103
111-113
: LGTM: Improved accessibility for the Width labelThe addition of the
for
attribute to the label element enhances accessibility by correctly associating it with the corresponding input field (id="frm_width"
). This change improves the user experience for those using assistive technologies.
129-131
: LGTM: Improved accessibility for multiple labelsThe addition of
for
attributes to the following labels enhances accessibility:
- Padding label (for="frm_label_padding")
- Color label in the Required Indicator section (for="frm_required_color")
- Weight label in the Required Indicator section (for="frm_required_weight")
These changes correctly associate the labels with their corresponding input fields, improving the user experience for those using assistive technologies.
Also applies to: 152-154, 170-172
Line range hint
1-189
: Summary: Significant accessibility improvements across the fileThe changes made to this file consistently enhance the accessibility of the form by adding
for
attributes to label elements and ensuring they correctly correspond to their associated input fields. These modifications align well with the PR objectives of improving accessibility features within the style editor elements.Key improvements:
- All label elements now have
for
attributes, properly linking them to their respective input fields.- The changes cover various form elements including color pickers, font size selectors, dropdowns, and sliders.
- The modifications improve the user experience for individuals using assistive technologies.
One minor suggestion was made regarding the consistency of the "Weight" label in the general settings section. Overall, these changes represent a significant step forward in making the form more accessible and user-friendly.
classes/views/styles/_general.php (11)
7-9
: LGTM: Accessibility improvement for Font Family labelThe addition of the
for
attribute to the label element is a positive change. It correctly associates the label with its corresponding input field (id="frm_font"), enhancing accessibility for users relying on assistive technologies.
31-35
: LGTM: Accessibility improvement for Alignment labelThe addition of the
for
attribute to the label element for Alignment is a positive change. It correctly associates the label with its corresponding input field (id="frm_form_align"), enhancing accessibility.
41-43
: LGTM: Added id parameter to FrmAlignStyleComponentThe addition of the 'id' parameter to the FrmAlignStyleComponent instantiation is correct. This change ensures that the component has the proper id ("frm_form_align") to match the label's
for
attribute, completing the accessibility improvement for the Alignment field.
48-52
: LGTM: Accessibility improvement for Border Color labelThe addition of the
for
attribute to the label element for Border Color is a positive change. It correctly associates the label with its corresponding input field (id="frm_fieldset_color"), enhancing accessibility for users relying on assistive technologies.
66-70
: LGTM: Accessibility improvement for Border Width labelThe addition of the
for
attribute to the label element for Border Width is a positive change. It correctly associates the label with its corresponding input field (id="frm_fieldset"), enhancing accessibility for users relying on assistive technologies.
84-88
: LGTM: Accessibility improvement for Padding labelThe addition of the
for
attribute to the label element for Padding is a positive change. It correctly associates the label with its corresponding input field (id="frm_fieldset_padding"), enhancing accessibility for users relying on assistive technologies.
103-107
: LGTM: Accessibility improvement for Form Width labelThe addition of the
for
attribute to the label element for Form Width is a positive change. It correctly associates the label with its corresponding input field (id="frm_form_width"), enhancing accessibility for users relying on assistive technologies.
121-125
: LGTM: Accessibility improvement for Direction label and componentThe changes made to the Direction section are positive:
- The addition of the
for
attribute to the label element correctly associates it with the input field (id="frm_direction").- The 'id' parameter added to the FrmDirectionStyleComponent instantiation ensures the component has the proper id to match the label's
for
attribute.These changes enhance accessibility for users relying on assistive technologies.
Also applies to: 131-133
139-141
: LGTM: Accessibility improvement for Override Theme labelThe addition of the
for
attribute to the label element for Override Theme is a positive change. It correctly associates the label with its corresponding input field (id="frm_important_style"), enhancing accessibility for users relying on assistive technologies.
161-165
: LGTM: Accessibility improvements for Center Form and Style Class labelsThe changes made to the Center Form and Style Class sections are positive:
- The addition of the
for
attribute to the Center Form label correctly associates it with the input field (id="frm_center_form").- The addition of the
for
attribute to the Style Class label correctly associates it with the input field (id="frm_style_class").These changes enhance accessibility for users relying on assistive technologies.
Also applies to: 180-184
Line range hint
1-184
: Summary: Excellent accessibility improvements across the fileThis pull request successfully enhances the accessibility of the style editor elements in the Formidable Forms plugin. The changes include:
- Adding
for
attributes to all<label>
elements, linking them to their corresponding input fields.- Ensuring that all related input fields and components have matching
id
attributes.These modifications significantly improve the usability of the form for users relying on assistive technologies, as they can now navigate more easily between labels and their associated input fields. The changes are consistent throughout the file and align perfectly with the PR objectives.
No issues or potential problems were identified in the implementation. Great job on improving the plugin's accessibility!
classes/views/styles/_quick-settings.php (8)
11-15
: Accessibility improvement: Label associationThe addition of the
for
attribute to the label element is a positive change. It improves accessibility by explicitly associating the label with its corresponding input field, which is crucial for users relying on assistive technologies.
41-45
: Consistent accessibility improvementThe addition of the
for
attribute to this label element is consistent with the previous change. It maintains the improved accessibility throughout the form, ensuring that all labels are properly associated with their input fields.
66-70
: Continued accessibility improvementsThe addition of the
for
attribute to the "Field Border" label maintains the pattern of accessibility improvements. This consistent approach ensures that all form elements are properly labeled and accessible.
89-93
: Comprehensive accessibility enhancementsThe addition of the
for
attribute to the "Button Text" label completes the set of accessibility improvements for the color-related form elements. These changes collectively enhance the form's usability for all users, particularly those relying on assistive technologies.
114-118
: Accessibility improvement and consistent ID namingThe addition of the
for
attribute to the "Vertical Spacing" label and the corresponding update to the input's ID improve accessibility. The ID naming convention (frm_field_margin
) is consistent with other non-color elements, maintaining a clear and logical structure.Also applies to: 125-125
135-139
: Consistent accessibility and ID naming improvementsThe changes to the "Input Field Padding" label and input maintain the pattern of accessibility improvements and consistent ID naming for non-color elements. This consistency helps maintain a clear structure and improves the overall accessibility of the form.
Also applies to: 146-146
156-160
: Accessibility improvements and new hidden inputThe changes to the "Base Font Size" label and input maintain the pattern of accessibility improvements and consistent ID naming.
However, I noticed a new hidden input field for "use_base_font_size" with a default value of "false". Could you please clarify the purpose of this field and how it interacts with the advanced settings?
Also applies to: 174-174
186-188
: Accessibility improvements and conditional displayThe changes to the "Field Shape" and "Corner Radius" labels and inputs continue the pattern of accessibility improvements and consistent ID naming. This maintains the overall enhanced accessibility of the form.
I noticed that the "Corner Radius" section is wrapped in a div with a "frm_hidden" class. This suggests that this section may be conditionally displayed, possibly based on the selected field shape. This approach allows for a dynamic and responsive user interface.
Also applies to: 203-205
classes/views/styles/_field-colors.php (13)
23-25
: LGTM: Improved accessibility for background color labelThe addition of the
for
attribute to the label element, linking it to the input with id "frm_bg_color", enhances accessibility. This change follows best practices by explicitly associating the label with its corresponding input field.
41-43
: LGTM: Enhanced accessibility for field text color labelThe addition of the
for
attribute to the label element, associating it with the input id "frm_text_color", improves accessibility. This change aligns with best practices for creating accessible web forms.
59-61
: LGTM: Accessibility improvement for border color labelThe
for
attribute has been correctly added to the label element, linking it to the input with id "frm_border_color". This change enhances the form's accessibility by ensuring proper association between the label and its corresponding input field.
77-79
: LGTM: Accessibility enhancement for border width labelThe addition of the
for
attribute to the label element, connecting it to the input with id "frm_field_border_width", improves the form's accessibility. This change ensures that screen readers can correctly associate the label with its input field.
95-97
: LGTM: Improved accessibility for border style labelThe
for
attribute has been appropriately added to the label element, linking it to the input with id "frm_field_border_style". This change enhances the form's accessibility by ensuring proper association between the label and its corresponding input field.
118-120
: LGTM: Enhanced accessibility for box shadow labelThe addition of the
for
attribute to the label element, associating it with the input id "frm_box_shadow", improves the form's accessibility. This change aligns with best practices for creating accessible web forms.
141-143
: LGTM: Accessibility improvement for active state background color labelThe
for
attribute has been correctly added to the label element, linking it to the input with id "frm_bg_color_active". This change enhances the form's accessibility by ensuring proper association between the label and its corresponding input field for the active state.
159-161
: LGTM: Enhanced accessibility for active state border color labelThe addition of the
for
attribute to the label element, connecting it to the input with id "frm_border_color_active", improves the form's accessibility. This change ensures that screen readers can correctly associate the label with its input field for the active state border color.
199-201
: LGTM: Accessibility enhancement for read-only state background color labelThe
for
attribute has been appropriately added to the label element, linking it to the input with id "frm_bg_color_disabled". This change improves the form's accessibility by ensuring proper association between the label and its corresponding input field for the read-only state background color.
217-219
: LGTM: Improved accessibility for read-only state field text color labelThe addition of the
for
attribute to the label element, associating it with the input id "frm_text_color_disabled", enhances the form's accessibility. This change follows best practices by explicitly linking the label to its corresponding input field for the read-only state field text color.
235-237
: LGTM: Enhanced accessibility for read-only state border color labelThe
for
attribute has been correctly added to the label element, linking it to the input with id "frm_border_color_disabled". This change improves the form's accessibility by ensuring proper association between the label and its corresponding input field for the read-only state border color.
257-259
: LGTM: Comprehensive accessibility improvements for Error state labelsThe changes in these line ranges consistently improve the accessibility of the form by adding
for
attributes to labels in the Error state. This includes labels for:
- Background color (lines 257-259)
- Field text color (lines 275-277)
- Border color (lines 293-295)
- Border width (lines 310-314, newly added)
- Border style (lines 329-331)
These modifications ensure proper associations between labels and their corresponding input fields, enhancing the form's usability for users relying on assistive technologies.
Also applies to: 275-277, 293-295, 310-314, 329-331
Line range hint
1-358
: Summary: Excellent accessibility improvements aligned with PR objectivesThis review has examined all the changes made to the
classes/views/styles/_field-colors.php
file. The modifications consistently improve the form's accessibility by addingfor
attributes to label elements, ensuring proper associations with their corresponding input fields. These changes cover all form states (Default, Active, Read Only, and Error) and various styling aspects (background color, text color, border properties, etc.).The implemented changes align perfectly with the PR objectives of enhancing accessibility features within the style editor elements. They contribute to a more inclusive user experience, particularly benefiting users who rely on assistive technologies.
Overall, these modifications represent a significant step forward in improving the accessibility of the Formidable Forms plugin. Great job on implementing these important accessibility enhancements!
classes/views/styles/_buttons.php (13)
37-39
: LGTM: Improved accessibility for font color labelThe addition of the
for
attribute to the label element enhances accessibility by explicitly associating it with the corresponding input field (frm_submit_text_color
). This change allows assistive technologies to correctly identify the relationship between the label and the input, improving the user experience for those using screen readers or other assistive devices.
54-58
: LGTM: Enhanced accessibility for font size labelThe addition of the
for
attribute to the label element for font size improves accessibility by creating an explicit association with the corresponding input field (frm_submit_font_size
). This change ensures that users of assistive technologies can easily understand and interact with the font size control.
73-75
: LGTM: Improved accessibility for weight labelThe addition of the
for
attribute to the label element for weight enhances accessibility by creating a clear association with the corresponding input field (frm_submit_weight
). This change improves the usability of the form for users relying on assistive technologies.
90-94
: LGTM: Enhanced accessibility for width labelThe addition of the
for
attribute to the label element for width improves accessibility by explicitly linking it to the corresponding input field (frm_submit_width
). This change ensures that users of assistive technologies can easily understand and interact with the width control.
109-113
: LGTM: Improved accessibility for height labelThe addition of the
for
attribute to the label element for height enhances accessibility by creating a clear association with the corresponding input field (frm_submit_height
). This change improves the usability of the form for users relying on assistive technologies.
128-130
: LGTM: Enhanced accessibility for border color labelThe addition of the
for
attribute to the label element for border color improves accessibility by explicitly linking it to the corresponding input field (frm_submit_border_color
). This change ensures that users of assistive technologies can easily understand and interact with the border color control.
145-149
: LGTM: Improved accessibility for border width labelThe addition of the
for
attribute to the label element for border width enhances accessibility by creating a clear association with the corresponding input field (frm_submit_border_width
). This change improves the usability of the form for users relying on assistive technologies.
164-166
: LGTM: Enhanced accessibility for shadow color labelThe addition of the
for
attribute to the label element for shadow color improves accessibility by explicitly linking it to the corresponding input field (frm_submit_shadow_color
). This change ensures that users of assistive technologies can easily understand and interact with the shadow color control.
181-185
: LGTM: Improved accessibility for corner radius labelThe addition of the
for
attribute to the label element for corner radius enhances accessibility by creating a clear association with the corresponding input field (frm_submit_border_radius
). This change improves the usability of the form for users relying on assistive technologies.
Line range hint
200-227
: LGTM: Enhanced accessibility for margin and padding labelsThe addition of
for
attributes to the label elements for margin (frm_submit_margin
) and padding (frm_submit_padding
) improves accessibility by explicitly linking them to their corresponding input fields. These changes ensure that users of assistive technologies can easily understand and interact with the margin and padding controls.The existing code between the changed lines, including the tooltip for the margin label, remains unaffected and continues to provide additional context for users.
243-245
: LGTM: Improved accessibility for disable styling labelThe addition of the
for
attribute to the label element for disabling submit button styling enhances accessibility by creating a clear association with the corresponding input field (frm_submit_style
). This change improves the usability of the form for users relying on assistive technologies, especially for this important toggle that affects the overall button styling.
Line range hint
270-366
: LGTM: Comprehensive accessibility improvements for hover and active statesThe addition of
for
attributes to the label elements for hover and active states of the submit button (background color, font color, and border color) significantly enhances accessibility. These changes create explicit associations between labels and their corresponding input fields:
- Hover state:
frm_submit_hover_bg_color
,frm_submit_hover_color
,frm_submit_hover_border_color
- Active state:
frm_submit_active_bg_color
,frm_submit_active_color
,frm_submit_active_border_color
The consistency in applying these accessibility improvements across different button states is commendable. This approach ensures a uniform and inclusive user experience for all form interactions, benefiting users who rely on assistive technologies.
Line range hint
1-384
: Overall: Excellent accessibility improvements throughout the fileThe changes made in this file demonstrate a comprehensive approach to enhancing accessibility in the form styling interface. By consistently adding
for
attributes to label elements and ensuring they correctly match their corresponding input field IDs, the code now provides a much more accessible experience for users relying on assistive technologies.These improvements cover various aspects of button styling, including:
- Default state properties (font color, size, weight, dimensions, borders, shadows, etc.)
- Hover state properties
- Active state properties
- Toggle for disabling button styling
The consistency in applying these changes across different form elements and button states is commendable and reflects a thorough understanding of accessibility best practices.
These enhancements will significantly improve the usability of the Formidable Forms plugin for all users, particularly those using screen readers or other assistive devices. Great job on prioritizing accessibility in this update!
classes/helpers/FrmStylesHelper.php (2)
Line range hint
1-1072
: Overall assessment: Approved with suggestions for future improvementsThe changes made to the
get_base_font_size_scale
method are beneficial and improve the code's robustness. However, there are several areas where the file could be improved:
- Address TODO comments to complete any unfinished features or documentation.
- Remove deprecated methods in future versions to maintain a clean codebase.
- Consider refactoring the class to better separate concerns and improve maintainability.
These suggestions aim to enhance the overall quality and maintainability of the codebase in the long term.
613-615
: Improved check for numeric values and zero division.The modification enhances the validation of input parameters in the
get_base_font_size_scale
method. It now checks if the$value
is numeric and not zero, preventing potential division by zero errors.This change improves the robustness of the method by adding an additional safety check. It's a good practice to validate input parameters, especially when they're used in mathematical operations.
To ensure this change doesn't introduce any unintended side effects, we should verify its usage across the codebase. Let's run a script to check for any calls to this method:
This will help us identify if there are any places where the method is called with potentially problematic arguments.
✅ Verification successful
Verification Complete: No Issues Found
The updated
get_base_font_size_scale
method has been successfully verified. The added checks effectively prevent potential division by zero errors, and the method is only called within theFrmStylesHelper.php
file, ensuring localized impact.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to get_base_font_size_scale method rg --type php "get_base_font_size_scale\s*\(" -A 3 -B 3Length of output: 1116
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.
I haven't tested this yet but it looks good to me.
I tagged Laura for QA as well. I'll merge once she approves.
Thanks @Liviu-p!
🚀
I'm going to go ahead and merge this. @lauramekaj1 if you do end up finding any issues, feel free to make a new issue in https://github.com/Strategy11/formidable-pro or drop a comment here. I think this is worth including for tomorrow either way as it shouldn't be introducing issues, but fixes a lot of accessibility issues. Thank you! |
Oh I commented too soon. I see you left comments in the other PR last week. Thank you for your feedback @lauramekaj1! It looks from that list that we're fine to merge this, and then continue making improvements. |
…n font size option, add extra check for base font size in order to avoid undefined when used as non-numeric
Fixes https://github.com/Strategy11/formidable-pro/issues/5382
Also fixes https://github.com/Strategy11/formidable-pro/issues/5391
Related Pro PR https://github.com/Strategy11/formidable-pro/pull/5400
This update adds missing
for
attributes to labels so clicking a label will focus the input (in the new visual styler editor).While also adding the missing font size setting for form descriptions as well.