Updated: Comment #0

Problem/Motivation

it is impossible to use keyboard tab navigation to tab through an admin page containing checkboxes or radios. Additionally, this is visually confusing when radios /checkboxes are interspersed with other elements like text boxes and select lists because as soon as you tab onto a radio/checkbox, you lose the visual highlight telling you where you are in the form.

Of course cross-browser consistency is also rearing its ugly head:

In Webkit browsers the standard blue "glow" outline style is used on most form elements that have focus. This is inconsistent with Firefox, which apparently is not using the blue glow style. The theme does make an effort to style some of the form elements by changing the border color to #ace (a light shade of blue), however because Webkit shows the additional outline, this border color change is all but lost next to the blue glow.

Additionally, in Webkit, radios and checkboxes don't appear to inherit this default glowing outline style because the actual input.form-radio and input.form-checkbox (to which the focused state and glow belongs) are set to be invisible with opacity: 0;.

Proposed resolution

Two things will bring this closer to cross-browser parity:

1) set outline: none; on focused form elements where border color is being changed. This will at least produce a more consistent look and feel across all modern browsers.

2) add two additional states to the sprite image used for radios and checkboxes: a "focused-unselected" state, and a "focused-selected" state.

The css might look something like this:

/* ---- in style.scss ---- */
input.form-text:focus,
input.form-file:focus,
textarea.form-textarea:focus,
select.form-select:focus {
  color: #000;
  border-color: #ace;
  outline: none; /* <-- add this line */
}

/* ----  in shiny.scss ---- */
input.form-checkbox:focus,
input.form-radio:focus {
  outline: none;
}
/* focused, unselected */
input.form-checkbox:focus + label::before {
  background-position: 0 -80px;
}
input.form-radio:focus + label::before {
  background-position: 0 -120px;
}
/* focused, selected */
input.form-checkbox:checked:focus + label::before {
  background-position: 0 -100px;
}
input.form-radio:checked:focus + label::before {
  background-position: 0 -140px;
}

Patch coming in first comment.

Remaining tasks

* Code review.
* Sanity check, because there is not a lot of inline documentation, I'm not sure what the sections of the code intended to fix radio/checkbox elements for webkit browsers only when they appear inside the form table layout are for. I've added the logic there too, but in my version of this has the unfortunate effect of *doubling* the toggle icon twice, and the second one is one pixel too high, so it looks bad (this could be a separate issue altogether).

User interface changes

Added focus styles for radios and checkboxes.

API changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jwilson3’s picture

Status: Active » Needs review
FileSize
12.66 KB
2.57 KB

This patch is formatted with git format-patch and contains the binary data for the toggles.png file that was modified, the patch can be applied using `git apply`.

I'm also attaching the png to the issue in case you want to see it:

Toggles

jwilson3’s picture

I've added a 1px stroke around all the elements using the color #ace but I'm not 100% happy with the result because the original stroke is maintained, and there seems to be some anti-aliasing issues especially noticeable around the round radio.

This could probably be improved, if a maintainer could provide the source file used to create this image.

Bojhan’s picture

@jwilson3 the source should be somewhere in the issue queue, I can probably dig it up. But I am not sure how useful the source psd is, exactly on stuff like this the browser is much more leading and photoshop is just dreaming.

jwilson3’s picture

@Bojhan:

on stuff like this the browser is much more leading and photoshop is just dreaming.

Maybe I'm misunderstanding, I agree that using default browser chrome is better, but this theme specifically does not do that, which kind of opens pandoras box to bugs like this.

I guess I'm confused at what your take away is. Is this something you would just rather not fix, and leave alone? If so, what is the reasoning?

Fwiw, I have my patch working great here, and will be using it on an upcoming distro that makes use of commerce and the shiny theme.

Bojhan’s picture

No, I am saying we should fix this - but that the photoshop files won't help much. Because they are an estimation on reality, its much easier to just design with reality :) aka. in the browser.

jwilson3’s picture

I need the PSD to change the 1 pixel border around the radio buttons, which do not look good on the version I made due to improper antialiasing problems. I scanned the issue queue but couldnt find any related issues that had attachments (i don't really know what I'm looking for, so it would be helpful if you could point me to it).

The psd will be useless if the radio button layer is already flattened in the source. Easier just to redo them from scratch.

jwilson3’s picture

While we're tweaking this we might as well create styles for *disabled* radios and checkboxes as well.

lsolesen’s picture

jwilson3’s picture

Ok, @bojhan, re: #5

its much easier to just design with reality :) aka. in the browser.

I think I finally understand that what you're saying is that these buttons were captured from some native browser output.

So the question is: which browser on which platform produced the radio and checkbox images?

dudenhofer’s picture

What I think Bojhan is referring to is using CSS for the borders/box shadows - so instead of updating the images, we should update the code to make it easier for others to adjust. I rolled another patch that does this. Any additional testing on this would be great. I've tested with Views, IEF, VBO, Rules, Taxonomy manager interfaces and have been trying to think of any others that might be affected.

I haven't thoroughly tested in IE yet, but might be able to address that specifically in this #2153101: Some checkboxes are missing in IE11

dudenhofer’s picture

Oops... i didn't have everything in that last patch. Re-uploading.

jwilson3’s picture

#10 & #11 add lots of more lines of CSS with vendor-specific hacks to have to maintain. It adds many more bytes of data than modifying the a single tiny png. One might argue that doing the design in the CSS is better because its retina-ready, but that doesn't apply here because we're still using PNGs for the actual images. I've thought about this, but cant really figure out what the benefit is of doing this change this way is. Please enlighten me!

Also, can you verify that the box shadow + border radius for the focused radio buttons actually works across all browsers, so that you don't get a square box shadow?

dudenhofer’s picture

One benefit is to make it easier for people to change - so they're not feeling like they need original PSD files to change a border color or box shadow. Another is that the rest of the theme uses rounded corners, box shadows, border radius, and background gradients - so why do these checkboxes differently? Do you have issues with the rounded corners and gradients on the buttons or select boxes or the overlays that are already in the theme?

What I verified before uploading this was that it would work on the browsers that the previous checkbox replacement worked on, so it wouldn't be a regression (the previous checkboxes didn't work perfectly across all browsers). But I was hoping to get another to verify this as well (the more the better).

I'm fine with reworking the background gradients (I realize I probably should have used the existing @mixin to generate those) and maybe using a vector check for those retina displays - even adding a better gradient png background to fall back on. I appreciate the quality check. But I feel like that is not the argument - the argument is about replacing these with CSS or png. In my opinion, just modifying the png is a smaller and quicker fix - but modifying these elements so they can be managed via CSS is a better fix.

dudenhofer’s picture

Here's an updated patch with some of the changes. I'm leaving the vector graphics for another issue cause there are more things that could benefit from that as well.

  • Commit 1f6d408 on 7.x-1.x by dudenhofer:
    Issue #2149123 by dudenhofer, jwilson3: Visually style checkboxes and...
dudenhofer’s picture

I went ahead and committed this, Just let me know if you notice any issues.

jwilson3’s picture

Thanks for the commit. Your logic in #13 is sound. I'll check this first chance I get back on the original (ongoing) project where we're using Shiny theme.

brahimmouhamou’s picture

What's the status here?
Is the commit valid?

jwilson3’s picture

Status: Needs review » Needs work

I believe the issue is almost resolved.

Two things:

  1. The toggles.png file is still in the codebase. This needs to be removed in favor of the new radio.png and check.png introduced in this issue.
  2. There is still one instance of 'toggles.png' being used in the sass/contrib.scss file:
    /* Edit Checkboxes only in Webkit which will render properly. */
    @media screen and (-webkit-min-device-pixel-ratio:0) {
      .commerce-backoffice-view .vbo-views-form {
        #edit-select .fieldset-wrapper {
          padding: 3px 0 0 20px; /* LTR */
        }
      }
      .vbo-select-all-markup {
        .vbo-fieldset-select-all {
          input.form-checkbox {
            opacity: 1;
            -webkit-appearance: none;
            -moz-appearance: none;
            height: 20px;
            width: 20px;
            background: transparent url("../images/toggles.png") no-repeat 0 0;
    

(sidenote: -moz-appearance:none is uselss in the snippet above on webkit-only media query, but thats a different issue ;)

I would also love to see this code abstracted into a mixin <-- nevermind, dont know what i was thinking when i wrote that, this was a mistake.

  • dudenhofer committed 21ed6be on 7.x-1.x
    Issue #2149123 by dudenhofer, jwilson3: Removed old toggles image and...
dudenhofer’s picture

Status: Needs work » Fixed

Nice catch on the toggle. I removed the image and updated the sass file in this last commit ^
As always, feel free to reopen if you notice anything else.
Thanks for taking the time to review!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.