-
-
Notifications
You must be signed in to change notification settings - Fork 821
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
Accessibility #4: Make alerts accessible, Added new setting under misc. #12158
Conversation
This PR relates to this work https://lab.civicrm.org/dev/accessibility/issues/4 |
settings/Core.setting.php
Outdated
'title' => 'Dismiss info alerts', | ||
'is_domain' => 1, | ||
'is_contact' => 0, | ||
'description' => 'If disabled, CiviCRM will stop dismissing the information alerts.', |
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.
Maybe instead, 'If disabled, CiviCRM will not automatically dismiss information alerts after 10 seconds.'
settings/Core.setting.php
Outdated
'is_domain' => 1, | ||
'is_contact' => 0, | ||
'description' => 'If disabled, CiviCRM will stop dismissing the information alerts.', | ||
'help_text' => NULL, |
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.
Could you put description in here so there is some on-screen explanation of what the setting does? Thx.
0bd7637
to
eaa1cd7
Compare
@JoeMurray We've updated the PR accordingly. To add on-screen explanation we had to put the description directly on |
js/Common.js
Outdated
@@ -1246,7 +1246,7 @@ if (!CRM.vars) CRM.vars = {}; | |||
unique: true | |||
}; | |||
options = $.extend(extra, options); | |||
options.expires = options.expires === false ? 0 : parseInt(options.expires, 10); | |||
options.expires = (options.expires === false || (!CRM.config.dismissInfoAlerts && type == "info")) ? 0 : parseInt(options.expires, 10); |
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.
There seems to be a logic error here. Automatic dismissal should be prevented for all errors, not just Info errors. I think you need to remove
&& type == "info"
Also, change name as per https://github.com/civicrm/civicrm-core/pull/12158/files#r191394619
settings/Core.setting.php
Outdated
'default' => TRUE, | ||
'html_type' => 'radio', | ||
'add' => '4.7', | ||
'title' => 'Dismiss info alerts', |
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 think success alerts expire at the moment too. What about saying 'Allow alerts to auto-dismiss?'.
Since we are affecting more than just info alerts, the name of the setting should change too. How about changing from dismiss_info_alerts to allow_alert_autodismissal ?
settings/Core.setting.php
Outdated
'title' => 'Dismiss info alerts', | ||
'is_domain' => 1, | ||
'is_contact' => 0, | ||
'description' => 'If disabled, CiviCRM will not automatically dismiss information alerts after 10 seconds.', |
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.
change 'information' to 'any'
settings/Core.setting.php
Outdated
'dismiss_info_alerts' => array( | ||
'group_name' => 'CiviCRM Preferences', | ||
'group' => 'core', | ||
'name' => 'dismiss_info_alerts', |
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.
<tr class="crm-miscellaneous-form-block-dismiss_info_alerts"> | ||
<td class="label">{$form.dismiss_info_alerts.label}</td> | ||
<td>{$form.dismiss_info_alerts.html}<br /> | ||
<p class="description">{ts}If disabled, CiviCRM will not automatically dismiss information alerts after 10 seconds.{/ts}</p> |
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.
change 'information' to 'any'.
@@ -479,6 +479,21 @@ function setting_getfields_expectedresult() { | |||
'description' => 'If enabled, CiviCRM will permit submissions from external sites to profiles. This is disabled by default to limit abuse.', | |||
'help_text' => '', | |||
), | |||
'dismiss_info_alerts' => array( |
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.
@@ -54,6 +54,7 @@ class CRM_Admin_Form_Setting_Miscellaneous extends CRM_Admin_Form_Setting { | |||
'recentItemsProviders' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, | |||
'dedupe_default_limit' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, | |||
'remote_profile_submissions' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, | |||
'dismiss_info_alerts' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, |
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.
CRM/Core/Resources.php
Outdated
@@ -690,6 +690,7 @@ public static function outputLocalizationJS() { | |||
'filters' => self::getEntityRefFilters(), | |||
), | |||
'ajaxPopupsEnabled' => self::singleton()->ajaxPopupsEnabled, | |||
'dismissInfoAlerts' => Civi::settings()->get('dismiss_info_alerts'), |
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.
'dismiss_info_alerts' => array( | ||
'group_name' => 'CiviCRM Preferences', | ||
'group' => 'core', | ||
'name' => 'dismiss_info_alerts', |
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.
'default' => '1', | ||
'html_type' => 'radio', | ||
'add' => '4.7', | ||
'title' => 'Dismiss info alerts', |
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.
'title' => 'Dismiss info alerts', | ||
'is_domain' => 1, | ||
'is_contact' => 0, | ||
'description' => 'If disabled, CiviCRM will stop dismissing the information alerts.', |
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.
How about, 'If disabled, alert messages in CiviCRM will never be automatically dismissed after 10 seconds.'
settings/Core.setting.php
Outdated
@@ -192,6 +192,21 @@ | |||
'description' => 'If enabled, CiviCRM will permit submissions from external sites to profiles. This is disabled by default to limit abuse.', | |||
'help_text' => NULL, | |||
), | |||
'dismiss_info_alerts' => array( |
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.
@@ -85,6 +85,12 @@ | |||
<p class="description">{ts}If enabled, CiviCRM will allow users to submit profiles from external sites. This is disabled by default to limit abuse.{/ts}</p> | |||
</td> | |||
</tr> | |||
<tr class="crm-miscellaneous-form-block-dismiss_info_alerts"> |
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.
@@ -85,6 +85,12 @@ | |||
<p class="description">{ts}If enabled, CiviCRM will allow users to submit profiles from external sites. This is disabled by default to limit abuse.{/ts}</p> | |||
</td> | |||
</tr> | |||
<tr class="crm-miscellaneous-form-block-dismiss_info_alerts"> | |||
<td class="label">{$form.dismiss_info_alerts.label}</td> |
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.
templates/CRM/common/l10n.js.tpl
Outdated
@@ -33,6 +33,7 @@ | |||
$.datepicker._defaults.dateFormat = CRM.config.dateInputFormat = {$config->dateInputFormat|@json_encode}; | |||
CRM.config.timeIs24Hr = {if $config->timeInputFormat eq 2}true{else}false{/if}; | |||
CRM.config.ajaxPopupsEnabled = {$ajaxPopupsEnabled|@json_encode}; | |||
CRM.config.dismissInfoAlerts = {$dismissInfoAlerts}; |
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.
Sorry for the verbose messages due to request for change in name. |
eaa1cd7
to
d19abfd
Compare
@JoeMurray We've updated the PR accordingly. |
I've tested and alert messages are being properly dismissed when new setting is enabled, and not dismissed when disabled. @colemanw I think this is ready for merging. |
Code looks great. Merging based on joe's testing. |
Overview
A problem for low vision users is alerts, their AT software doesn't recognise them, so they miss warnings and end up adding duplicates. It would be great to have some sort of per user control for setting how alerts display, perhaps even adding audio cues.
Before
The CiviCRM alerts were not accessible for AT software and there was no setting to disable the dismiss of the info alerts.
After
Now the alerts are accessible which means screen reader software can be set to provide audible alerts, and we have a setting under civicrm/admin/setting/misc to disable the dismissal of all alerts.
Comments
Agileware Ref: CIVICRM-856