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

Add global setting for form submission message #1615

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

AbdiTolesa
Copy link
Contributor

@AbdiTolesa AbdiTolesa commented Apr 11, 2024

Fix https://github.com/Strategy11/formidable-pro/issues/3739

This PR adds a feature that announces a form submission message with screen reader when a form is submitted. There is also a new global setting added for updating the message.

image

Test steps:

  1. Go to the global settings and check that a new setting 'Submit Speak Text' is there.
  2. Modify it as needed for your test.
  3. Turn on a screen reader and try submitting a form.
  4. Confirm that the message is announced by your screen reader.

Copy link
Contributor

coderabbitai bot commented Apr 11, 2024

Walkthrough

The changes introduce a new feature to handle form submission announcements in the Formidable Forms plugin. A new parameter for submission speech messages is added, enabling form accessibility improvement through auditory feedback. This feature includes the addition of a speak method in JavaScript, a new static method for retrieving messages in PHP, and an input field in the settings form for customizing the speak text.

Changes

Files Change Summary
classes/helpers/FrmAppHelper.php Added new parameter submit_speak_msg to localize_script method including a call to FrmFormsHelper::get_submit_speak_message()
classes/helpers/FrmFormsHelper.php Introduced get_submit_speak_message() method to retrieve and customize the submission announcement message
classes/models/FrmSettings.php Added public property $submit_speak_msg to store the default form submission message
classes/views/frm-settings/messages.php Added input field for "Submit Speak Text" in the settings form with label and tooltip
js/formidable.js Updated frmFrontFormJS function to include a speak method for auditory feedback and modified submitForm method to call speak functionality

Sequence Diagrams

sequenceDiagram
    participant User
    participant FrmSettings
    participant FrmFormsHelper
    participant Form
    participant JavaScript

    User->>FrmSettings: Open settings
    FrmSettings->>User: Display settings form
    User->>FrmSettings: Update "Submit Speak Text"
    FrmSettings->>FrmFormsHelper: Save $submit_speak_msg

    User->>Form: Submit form
    Form->>JavaScript: Trigger form submission
    JavaScript->>JavaScript: Call "speak" method with message
    JavaScript->>User: Announce form submission message
    JavaScript->>Form: Proceed with form submission
Loading

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5d4f16a and d3c4233.

Files selected for processing (3)
  • classes/helpers/FrmAppHelper.php (1 hunks)
  • classes/helpers/FrmFormsHelper.php (1 hunks)
  • js/formidable.js (1 hunks)
Additional context used
Biome
js/formidable.js

[error] 5-5: Redundant use strict directive. (lint/suspicious/noRedundantUseStrict)

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.


[error] 70-72: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 504-510: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 518-520: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 513-521: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 700-728: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 638-798: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 800-803: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1046-1048: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1103-1111: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1169-1173: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1197-1199: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1187-1200: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1225-1230: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1232-1234: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1265-1265: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1279-1304: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1307-1314: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1306-1315: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1321-1323: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

Additional comments not posted (3)
js/formidable.js (1)

1557-1570: Implement the new speak method to announce messages to screen readers.

This method correctly creates a temporary hidden div with the aria-live="assertive" attribute, ensuring that the message is immediately announced by screen readers. The use of setTimeout with a delay of 0 to remove the element after it has been announced is an appropriate technique to ensure the message is read without keeping the element in the DOM.

classes/helpers/FrmFormsHelper.php (1)

272-276: Verify the usage of the frm_submit_speak_message filter across the plugin to ensure it does not unintentionally alter functionality elsewhere.

Verification successful

The filter frm_submit_speak_message is used only once in the codebase, within the FrmFormsHelper.php file. This indicates that its impact is minimal and contained. To ensure clarity and proper usage, it is recommended to document this filter in the WordPress plugin's developer documentation.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Search for the usage of 'frm_submit_speak_message' filter across the PHP files.
rg --type php "apply_filters\\(\\s*'frm_submit_speak_message'"

Length of output: 195

classes/helpers/FrmAppHelper.php (1)

3289-3289: Consider removing the trailing tilde character on line 3289.

+3289

Likely invalid or redundant comment.


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

js/formidable.js Outdated
Comment on lines 1657 to 1671
speak: function( message ) {
var element = document.createElement( 'div' );
element.id = 'speak_formidable_form_submitted';
element.setAttribute( 'aria-live', 'assertive' );
element.className = 'frm_screen_reader frm_hidden';
element.textContent = message;
document.body.appendChild( element );

setTimeout(
function() {
document.body.removeChild( element );
},
1000
);
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor the speak method to manage the temporary element more robustly, possibly by checking if the element still exists before attempting to remove it.

@@ -3218,6 +3218,7 @@ public static function localize_script( $location ) {
'empty_fields' => __( 'Please complete the preceding required fields before uploading a file.', 'formidable' ),
'focus_first_error' => self::should_focus_first_error(),
'include_alert_role' => self::should_include_alert_role_on_field_errors(),
'submit_speak_msg' => ( new FrmSettings() )->submit_speak_msg,
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the trailing tilde character.

- 3221~
+ 3221

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
'submit_speak_msg' => ( new FrmSettings() )->submit_speak_msg,
'submit_speak_msg' => ( new FrmSettings() )->submit_speak_msg,

@AbdiTolesa AbdiTolesa requested a review from garretlaxton April 11, 2024 08:57
Copy link

codecov bot commented Apr 11, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 29.58%. Comparing base (df4e249) to head (5667591).
Report is 196 commits behind head on master.

Current head 5667591 differs from pull request most recent head d3c4233

Please upload reports for the commit d3c4233 to get more accurate results.

Files Patch % Lines
classes/helpers/FrmAppHelper.php 0.00% 1 Missing ⚠️
classes/models/FrmSettings.php 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1615      +/-   ##
============================================
+ Coverage     27.46%   29.58%   +2.11%     
+ Complexity     7916     7858      -58     
============================================
  Files           125      121       -4     
  Lines         26445    25954     -491     
============================================
+ Hits           7264     7678     +414     
+ Misses        19181    18276     -905     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@garretlaxton garretlaxton left a comment

Choose a reason for hiding this comment

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

I am guessing this should get cut off when the form request is complete? I hear the text, but it stops after the form request is complete.

@AbdiTolesa
Copy link
Contributor Author

I am guessing this should get cut off when the form request is complete? I hear the text, but it stops after the form request is complete.

@garretlaxton I think you mistyped some words in your comment probably? I didn't get it well.

@Crabcyborg
Copy link
Contributor

@AbdiTolesa It sounds like the page exits before the message is read.

I think we may be handling this from the wrong angle. I don't think we need to announce the submit event, but rather, the success event.

If the confirmation action is a message, I think we could probably just add aria-live to the success message, then that will be read out.

If there is a redirect action, we're probably fine to just not speak/add anything.

@AbdiTolesa
Copy link
Contributor Author

If the confirmation action is a message, I think we could probably just add aria-live to the success message, then that will be read out.

@Crabcyborg Currently it looks the success message is announced without extra update, at least that's the default behaviour when i test with VoiceOver. But quoting the original issue description, I think the intention was more about the phase after Submit button is clicked when the form is being processed (spinner loading).

A blind user doesn't see the spinner when a submit button is loaded. We should add an aria-label or use a speak function

@Crabcyborg
Copy link
Contributor

Thanks @AbdiTolesa!

Maybe we can go with a shorter message. What you have is really long.

I think just "Form submitted" is probably enough.

@AbdiTolesa
Copy link
Contributor Author

Thanks @AbdiTolesa!

Maybe we can go with a shorter message. What you have is really long.

I think just "Form submitted" is probably enough.

Should I keep the new global setting to customise that or we should hard code it? Or probably make it customisable with a new filter?

@Crabcyborg
Copy link
Contributor

@AbdiTolesa I think just a new filter is fine. This feature doesn't need a lot of visibility.

@AbdiTolesa
Copy link
Contributor Author

@AbdiTolesa I think just a new filter is fine. This feature doesn't need a lot of visibility.

I have updated my code to just use a new filter.

@AbdiTolesa AbdiTolesa requested a review from Crabcyborg April 15, 2024 18:49
js/formidable.js Outdated Show resolved Hide resolved
@AbdiTolesa AbdiTolesa requested a review from Crabcyborg April 16, 2024 06:13
@AbdiTolesa AbdiTolesa requested review from Crabcyborg and removed request for Crabcyborg June 5, 2024 06:53
@AbdiTolesa AbdiTolesa requested a review from truongwp June 18, 2024 14:16
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
js/formidable.js (1)

Line range hint 5-5: Remove the redundant "use strict" directive.

As modules are strict by default, this directive is unnecessary and can be safely removed to clean up the code.

- 'use strict';

Comment on lines +263 to +278
/**
* Returns the message announced when a form is submitted.
*
* @since x.x
*
* @return string
*/
public static function get_submit_speak_message() {
/**
* Allows updating a message announced when a form is submitted.
*
* @since x.x
* @param string $submit_speak_message
*/
return apply_filters( 'frm_submit_speak_message', __( 'Form submitted', 'formidable' ) );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure the @since tag in the get_submit_speak_message method is updated to reflect the actual version being released.

- * @since x.x
+ * @since 4.05  // Example version, replace with the actual version being released
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Returns the message announced when a form is submitted.
*
* @since x.x
*
* @return string
*/
public static function get_submit_speak_message() {
/**
* Allows updating a message announced when a form is submitted.
*
* @since x.x
* @param string $submit_speak_message
*/
return apply_filters( 'frm_submit_speak_message', __( 'Form submitted', 'formidable' ) );
}
/**
* Returns the message announced when a form is submitted.
*
* @since 4.05 // Example version, replace with the actual version being released
*
* @return string
*/
public static function get_submit_speak_message() {
/**
* Allows updating a message announced when a form is submitted.
*
* @since x.x
* @param string $submit_speak_message
*/
return apply_filters( 'frm_submit_speak_message', __( 'Form submitted', 'formidable' ) );
}

Copy link
Contributor

@truongwp truongwp left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks @AbdiTolesa!

@AbdiTolesa AbdiTolesa requested review from Crabcyborg and removed request for Crabcyborg June 19, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants