-
-
Notifications
You must be signed in to change notification settings - Fork 815
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
[REF] Fix empty link accessibility issues in nav menu by setting scre… #30682
base: master
Are you sure you want to change the base?
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
@seamuslee001 does this PR provides a alternative fixes to https://github.com/civicrm/civicrm-core/pull/30676/files ? |
@monishdeb the first part i think is redundant with your PR the 2nd part of the patch seems to be needed because I see |
…en reader only span content Remove part not needed due to monish's patch
20ad570
to
244b293
Compare
I see can you also include this portion on your PR : diff --git a/js/crm.menubar.js b/js/crm.menubar.js
index 357331f382f..fa190fdd139 100644
--- a/js/crm.menubar.js
+++ b/js/crm.menubar.js
@@ -287,6 +287,7 @@
initializeSearch: function() {
$('input[name=qfKey]', '#crm-qsearch').attr('value', CRM.menubar.qfKey);
$('#crm-qsearch-input')
+ .attr('title', ts('Name/Email'))
.autocomplete({
source: function(request, response) { so that I can close #30676 ? |
No these are complimentary now hopefully |
@@ -482,7 +482,7 @@ | |||
'</li>', | |||
drillTpl: | |||
'<li class="crm-menu-border-bottom" data-name="MenubarDrillDown">' + | |||
'<a href="#"><input type="text" id="crm-menubar-drilldown" placeholder="' + _.escape(ts('Find menu item...')) + '"></a>' + | |||
'<a href="#"><input type="text" id="crm-menubar-drilldown" placeholder="' + _.escape(ts('Find menu item...')) + '"><span class="sr-only">' + _.escape(ts('Find menu item...')) + '></a>' + |
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.
@seamuslee001 can you add closing </span>
tag? that is missing here.
test this please |
As @monishdeb says, this lacks a closing span but the browser seems to add it, I think because it's wrapped in an This only fixes one of the two empty link errors on the navbar for me now, the first search box still gives one: I think one of the fixes in the now closed PR from Monish (maybe this line?) would resolve that. |
No description provided.