Ticket #6264 (closed defect: fixed)

Opened 4 months ago

Last modified 2 months ago

Toolbar: extra spacing between buttons (FF)

Reported by: jeffg Owned by: sfoster
Priority: normal Milestone: 1.2
Component: Dijit - LnF Version: 1.0
Severity: normal Keywords:
Cc:

Description (last modified by alex) (diff)

Based on unchanged markup, recent builds as we approach 1.1 have introduced extra space between buttons on toolbars. Both on custom markup (that haveabsolutely no space in it), and in the Editor tool bar, when invoked through markup.

It badly screws up the look of things...

Proper look on IE7:

Proper look on IE

Extra spacing on FF2:

Improper look on FF2.x (windows)

Attachments

toolbar.png (8.0 kB) - added by bill 4 months ago.
IEButtons.png (1.8 kB) - added by jeffg 4 months ago.
Proper look on IE
FFButtons.png (1.9 kB) - added by jeffg 4 months ago.
Improper look on FF2.x (windows)
patch6264.diff (1.1 kB) - added by dylan 3 months ago.
6264_mozFocusInner.patch (288 bytes) - added by sfoster 3 months ago.
[PATCH] [CLA] fixes a11y regression with the -moz-focus-inner rule by removing border:none (dijit.css)

Change History

  Changed 4 months ago by bill

  • priority changed from highest to low
  • owner deleted
  • component changed from General to Dijit
  • milestone changed from 1.1.1 to 1.2

They look fine to me. I'm attaching an image... does it look different for you?

Changed 4 months ago by bill

Changed 4 months ago by jeffg

Proper look on IE

Changed 4 months ago by jeffg

Improper look on FF2.x (windows)

  Changed 4 months ago by jeffg

  • priority changed from low to high
  • milestone changed from 1.2 to 1.1.1

Attached are the images the editor from our app under IE7 and FF2 (windows). The same problem occurs in our app with buttons created from markup on dojo toolbar. Same code & markup with different results.

It is a very broken look on FF, and it is a poor look as well, hence the upped priority.

  Changed 4 months ago by jeffg

  • summary changed from Button inTtoolbars have exta space to Buttons inToolbars have exta space

  Changed 4 months ago by jeffg

  • summary changed from Buttons inToolbars have exta space to Buttons inToolbars have extra space

  Changed 4 months ago by bill

  • priority changed from high to normal
  • summary changed from Buttons inToolbars have extra space to Toolbar: extra spacing between buttons (FF)
  • description modified (diff)
  • milestone changed from 1.1.1 to 1.2

Yes, there's a problem on FF2 and FF3 where the <span> node w/the icon is 18x18 but the button itself is 24x20, although according to firebug neither the button nor the span have padding or margin... you can workaround this by just adding a CSS rule to your main page:

.dijitToolbar .dijitButton {
	margin: 0px;
	width: 18px;
	padding-left: -3px;
}

  Changed 4 months ago by bill

  • component changed from Dijit to Dijit - LnF

  Changed 4 months ago by alex

  • description modified (diff)

bill: really unsure why this can't land on 1.1.1. What's the rationale for punting?

  Changed 3 months ago by dylan

Culprit is [12788]

  .nihilo .dijitToolbar { 
 padding: 3px 0 1px 3px; 

etc.

Will attach a patch.

Changed 3 months ago by dylan

  Changed 3 months ago by dylan

  • status changed from new to assigned
  • owner set to dylan

  Changed 3 months ago by dylan

  • milestone changed from 1.2 to 1.1.1

  Changed 3 months ago by dylan

  • status changed from assigned to closed
  • resolution set to fixed

(In [13389]) fixes #6264, remove extra spacing from editor toolbar icons

  Changed 3 months ago by dylan

(In [13390]) refs #6264, apply patch to 1.1 branch for Dojo 1.1.1

  Changed 3 months ago by dylan

  • status changed from closed to reopened
  • resolution deleted

follow-up: ↓ 15   Changed 3 months ago by dylan

Patch from earlier helps, but not enough. Need to look at this again.

in reply to: ↑ 14   Changed 3 months ago by sfoster

Replying to dylan:

Patch from earlier helps, but not enough. Need to look at this again.

the button element in FF has some default (non-removable) padding (3px IIRC). So any solution to this will either be some css hack (thats not coming to me right now), or changing to use some other element - which may or may not be a solution. Here's some details and screenshots: http://www.designdetector.com/demos/buttons-padding-demo.html

Using negative padding on the button doesnt do anything for me.

  Changed 3 months ago by peller

(In [13578]) Back out Dylan's changes from [13390] on 1.1 branch. Refs #6264

  Changed 3 months ago by bill

  • milestone changed from 1.1.1 to 1.3

  Changed 3 months ago by sfoster

  • status changed from reopened to new
  • owner changed from dylan to sfoster
  • milestone changed from 1.3 to 1.2

From a snippet on http://www.sitepoint.com/forums/showthread.php?t=547059

6264_mozFocusInner.patch adds a rule to the tundra toolbar buttons that effectively removes the mysterious inner padding. The same could/should be applied to the other themes presumably - or perhaps this belongs in dijit.css? I dont know to what extent we've designed around the current behavior.

Moving back to 1.2, unless someone feels strongly that its needs to be squeezed into 1.1.1

  Changed 3 months ago by sfoster

  • status changed from new to assigned

Re-created the 6264_mozFocusInner.patch to patch dijit.css and fix this for all themes

  Changed 3 months ago by bill

  • status changed from assigned to closed
  • resolution set to fixed

(In [13617]) Fix toolbar spacing on FF. Patch from Sam Foster (CLA on file). Fixes #6264.

Changed 3 months ago by sfoster

[PATCH] [CLA] fixes a11y regression with the -moz-focus-inner rule by removing border:none (dijit.css)

  Changed 3 months ago by sfoster

  • status changed from closed to reopened
  • resolution deleted

Turns out that removing the border specified by -moz-focus-inner (in the mozilla/ff default stylesheet) prevents the button from accepting focus. I updated the 624_mozFocusInner.patch (already committed to trunk) to remove that property assignment. This inner border basically is the outline -which we want. The padding is still removed, so I still consider this fixed. I'll look into seeing if there's a way to claw back the 2px.

  Changed 3 months ago by dante

(In [13623]) refs #6264 - minor wrinkle in patch to fix padding regressed a11y. fix before becka11y notices :) (courtesy Sam Foster, CLA)

  Changed 2 months ago by dante

  • status changed from reopened to closed
  • resolution set to fixed
Note: See TracTickets for help on using tickets.