Accessibility issues

[Deklin]

Hi there, a couple of issues we are working to resolve

1. When using picker mode, that should have the default focus so you can tab through each of the items
2. Enter should select the item, shouldn't require a mouse click.

Once I have a fix i'll reply to this if you want to merge.

[Deklin]

Here is the fix I've done. Marked with // BEGIN PATCH and // END PATCH

init: function(type, select, options) {
      var self = this;

      self.type = type;

      self.$select = $(select);
      self.$select.hide();

      self.options = $.extend({}, $.fn.simplecolorpicker.defaults, options);

      self.$colorList = null;

      if (self.options.picker === true) {
        var selectText = self.$select.find('> option:selected').text();
        self.$icon = $('<span class="simplecolorpicker icon"'
                     + ' title="' + selectText + '"'
                     + ' style="background-color: ' + self.$select.val() + ';"'
                     + ' role="button" tabindex="0">'
                     + '</span>').insertAfter(self.$select);
        self.$icon.on('click.' + self.type, $.proxy(self.showPicker, self));
        // BEGIN PATCH
        self.$icon.on('keypress.' + self.type, $.proxy(self.showPicker, self));
        // END PATCH

        self.$picker = $('<span class="simplecolorpicker picker ' + self.options.theme + '"></span>').appendTo(document.body);
        self.$colorList = self.$picker;

        // Hide picker when clicking outside
        $(document).on('mousedown.' + self.type, $.proxy(self.hidePicker, self));
        self.$picker.on('mousedown.' + self.type, $.proxy(self.mousedown, self));
      } else {
        self.$inline = $('<span class="simplecolorpicker inline ' + self.options.theme + '"></span>').insertAfter(self.$select);
        self.$colorList = self.$inline;
      }

      // Build the list of colors
      // <span class="color selected" title="Green" style="background-color: #7bd148;" role="button"></span>
      self.$select.find('> option').each(function() {
        var $option = $(this);
        var color = $option.val();

        var isSelected = $option.is(':selected');
        var isDisabled = $option.is(':disabled');

        var selected = '';
        if (isSelected === true) {
          selected = ' data-selected';
        }

        var disabled = '';
        if (isDisabled === true) {
          disabled = ' data-disabled';
        }

        var title = '';
        if (isDisabled === false) {
          title = ' title="' + $option.text() + '"';
        }

        var role = '';
        if (isDisabled === false) {
          role = ' role="button" tabindex="0"';
        }

        var $colorSpan = $('<span class="color"'
                         + title
                         + ' style="background-color: ' + color + ';"'
                         + ' data-color="' + color + '"'
                         + selected
                         + disabled
                         + role + '>'
                         + '</span>');

        self.$colorList.append($colorSpan);
        $colorSpan.on('click.' + self.type, $.proxy(self.colorSpanClicked, self));
        // BEGIN PATCH
        $colorSpan.on('keypress.' + self.type, $.proxy(self.colorSpanClicked, self));
        // END PATCH

        var $next = $option.next();
        if ($next.is('optgroup') === true) {
          // Vertical break, like hr
          self.$colorList.append('<span class="vr"></span>');
        }
      });
    },



    /**
     * The user clicked on a color inside $colorList.
     */
    colorSpanClicked: function(e) {
      // BEGIN PATCH
      if (e.type !== 'click' && (e.type !== 'keypress' && e.which !== 13)) {
        return;
      }
      // END PATCH

      // When a color is clicked, make it the new selected one (unless disabled)
      if ($(e.target).is('[data-disabled]') === false) {
        this.selectColorSpan($(e.target));
        this.$select.trigger('change');
      }            
    },



 showPicker: function(e) {
      // BEGIN PATCH
      if (e.type !== 'click' && (e.type !== 'keypress' && e.which !== 13)) {
        return;
      }
      // END PATCH

      var pos = this.$icon.offset();
      // BEGIN PATCH
      var $firstButton, $lastButton;
      // END PATCH

      this.$picker.css({
        // Remove some pixels to align the picker icon with the icons inside the dropdown
        left: pos.left - 6,
        top: pos.top + this.$icon.outerHeight()
      });

      this.$picker.show(this.options.pickerDelay);


      // BEGIN PATCH
      // This sets the focus to the first button in the picker dialog
      // It also enables looping of the tabs both forward and reverse direction.      
      $firstButton = this.$picker.find('[role=button]').first();
      $lastButton = this.$picker.find('[role=button]').last();

      $firstButton.focus();

      $firstButton.on('keydown', function(e) {
        if (e.which === 9 && e.shiftKey) {
          e.preventDefault();
          $lastButton.focus();
        }
      });

      $lastButton.on('keydown', function(e) {
        if (e.which === 9 && !e.shiftKey) {
           e.preventDefault();
           $firstButton.focus();
        }
      });            
      // END PATCH         
    },

This patch allows you to press enter or click to select
It also focuses the picker dialog and you can tab through (tab will also wrap)
you can press enter to select there as well as a mouse click.

[Deklin]

I tried to create a branch to issue a pull request but it appears you may have branch off.

[tkrotoff]

I've run your patch and it is a very good! Thanks.

As for the pull request, I did not disable anything. I've already gotten several pull requests

I'm wondering if the loop for the buttons inside the picker dialog/span is needed. What about setting the focus on the picker dialog/span and then let the web browser do the rest? (I did not try this).

[Deklin]

Hi there, I'm a newbie to Git so it's possible I setup something wrong. I
tried a git push origin patches/deklin but keep getting I don't have
access. Even tried with an SSH origin and here is the message github
responded to me

[image: Inline image 1]

My guess is I should have cloned it first. I'll try that.

Regarding the code--

The loop was needed because when you tabbed to the last button, the next
tab would go outside the dialog with no way to get back into it.

There may be a better way to handle this but this was a pattern I've used
before so I figured i'd add it. That way in the dialog itself (the picker)
you can tab in there safely both directions.

I didn't test tabbing with an inline and no picker however so we may need
to limit that wrapping to the picker only and not inline.

On Fri, Nov 8, 2013 at 2:13 PM, Tanguy Krotoff notifications@gh.neting.ccwrote:

I've run your patch and it is a very good! Thanks.

As for the pull request, I did not disable anything. I've already gotten
several pull requestshttps://github.com/tkrotoff/jquery-simplecolorpicker/pulls?direction=desc&page=1&sort=created&state=closed

I'm wondering if the loop for the buttons inside the picker dialog/span is
needed. What about setting the focus on the picker dialog/span and then let
the web browser do the rest? (I did not try this).

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/18#issuecomment-28089192
.

[Deklin]

I have a new patch that streamlines things a bit more. Working on finding out why I can't branch to create a pull request...

It also adds ESC to close picker and re-focuses $icon