The close button generated by the tour module looks like this:

<a href="#close" class="joyride-close-tip">&times;</a>

It lacks descriptive text and does not provide a way for screenreader users to know what the button is for.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

keeganstreet created an issue. See original summary.

keeganstreet’s picture

rli’s picture

Status: Active » Needs review
nick_schuch’s picture

Thanks for the patch keeganstreet!

You will need to modify the "tour.es6.js" instead as per https://www.drupal.org/node/2815083

nick_schuch’s picture

Status: Needs review » Needs work
keeganstreet’s picture

Thanks @nick_schuch, I'm adding a new patch now for 8.4.x which includes the change in the ES6 source file.

mgifford’s picture

Version: 8.2.x-dev » 8.4.x-dev
Status: Needs work » Needs review
Sumit kumar’s picture

Status: Needs review » Reviewed & tested by the community

Hi Apply patch #6 on 8.4.x and its applied successfully.
working fine

larowlan’s picture

Looks good to me

larowlan’s picture

Issue tags: +JavaScript
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/tour/js/tour.es6.js
@@ -155,7 +155,7 @@
-              link: '<a href=\"#close\" class=\"joyride-close-tip\">&times;</a>',
+              link: '<a href=\"#close\" class=\"joyride-close-tip\" aria-label=\"Close\">&times;</a>',

+++ b/core/modules/tour/js/tour.js
@@ -76,7 +76,7 @@
-              link: '<a href=\"#close\" class=\"joyride-close-tip\">&times;</a>',
+              link: '<a href=\"#close\" class=\"joyride-close-tip\" aria-label=\"Close\">&times;</a>',

We need to translate these with Drupal.t()

Dinesh18’s picture

Status: Needs work » Needs review
FileSize
1.18 KB

Here is an updated patch with Drupal.t()

larowlan’s picture

Status: Needs review » Needs work
+++ b/core/modules/tour/js/tour.es6.js
@@ -155,7 +155,7 @@
+              link: Drupal.t('<a href=\"#close\" class=\"joyride-close-tip\">&times;</a>'),

+++ b/core/modules/tour/js/tour.js
@@ -76,7 +76,7 @@
+              link: Drupal.t('<a href=\"#close\" class=\"joyride-close-tip\">&times;</a>'),

We lost the aria-label attributes.

We shouldn't be translating the whole string, just the 'Close' text.

So, probably best to evaluate Drupal.t('Close') into a variable, and then use string concatenation + in JavaScript to use the translated text with the existing one.

Gábor Hojtsy’s picture

$ git grep “t(‘Close’”
core/modules/contextual/js/contextual.es6.js:        close: Drupal.t(‘Close’)
core/modules/contextual/js/contextual.js:      close: Drupal.t(‘Close’)
core/modules/quickedit/js/views/EntityToolbarView.es6.js:              label: Drupal.t(‘Close’),
core/modules/quickedit/js/views/EntityToolbarView.js:          label: Drupal.t(‘Close’),

Looked if we have any prior art with context (because "close" could mean "nearby" as well :D) but apparently not, so we can assume people already translate this as “Finish” not “Nearby”.

Dinesh18’s picture

    toggleTour: function toggleTour() {
      if (this.model.get('isActive')) {
        var $tour = this._getTour();
        this._removeIrrelevantTourItems($tour, this._getDocument());
        var that = this;
		var $close = Drupal.t('Close');
        if ($tour.find('li').length) {
          $tour.joyride({
            autoStart: true,
            postRideCallback: function postRideCallback() {
              that.model.set('isActive', false);
            },

            template: {
              link: '<a href=\"#close\" class=\"joyride-close-tip\" aria-label=\"' . $close . '\">&times;</a>',
              button: '<a href=\"#\" class=\"button button--primary joyride-next-tip\"></a>'
            }
          });
          this.model.set({ isActive: true, activeTour: $tour });
        }
      } else {
        this.model.get('activeTour').joyride('destroy');
        this.model.set({ isActive: false, activeTour: [] });
      }
    },

added as per #13. Is it good so that I can create the patch

larowlan’s picture

Hi @Dinesh18 in Javscript, use + not . to concatenate.

Also, use close, not $close, as its a plain variable - not a jQuery reference

Dinesh18’s picture

Thanks larowlan. Here is an updated patch.

Dinesh18’s picture

Status: Needs work » Needs review
rogierbom’s picture

Issue tags: +sprint
droplet’s picture

Status: Needs review » Needs work
Issue tags: +Needs JS testing

Let's clean up the ugly concatenation with ES6 template literals and drop the var.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

naiduharish’s picture

Status: Needs work » Needs review
FileSize
1.7 KB

Modified patch by using template literal and const.