Opened 12 years ago

Closed 11 years ago

#6264 closed defect (fixed)

Toolbar: extra spacing between buttons (FF)

Reported by: jeffg Owned by: Sam Foster
Priority: high Milestone: 1.2
Component: Dijit - LnF Version: 1.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by alex)

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 (5)

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

Download all attachments as: .zip

Change History (28)

comment:1 Changed 12 years ago by bill

Component: GeneralDijit
Milestone: 1.1.11.2
Owner: anonymous deleted
Priority: highestlow

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

Changed 12 years ago by bill

Attachment: toolbar.png added

Changed 12 years ago by jeffg

Attachment: IEButtons.png added

Proper look on IE

Changed 12 years ago by jeffg

Attachment: FFButtons.png added

Improper look on FF2.x (windows)

comment:2 Changed 12 years ago by jeffg

Milestone: 1.21.1.1
Priority: lowhigh

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.

comment:3 Changed 12 years ago by jeffg

Summary: Button inTtoolbars have exta spaceButtons inToolbars have exta space

comment:4 Changed 12 years ago by jeffg

Summary: Buttons inToolbars have exta spaceButtons inToolbars have extra space

comment:5 Changed 12 years ago by bill

Description: modified (diff)
Milestone: 1.1.11.2
Priority: highnormal
Summary: Buttons inToolbars have extra spaceToolbar: extra spacing between buttons (FF)

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;
}

comment:6 Changed 12 years ago by bill

Component: DijitDijit - LnF

comment:7 Changed 12 years ago by alex

Description: modified (diff)

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

comment:8 Changed 11 years ago by dylan

Culprit is [12788]

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

etc.

Will attach a patch.

Changed 11 years ago by dylan

Attachment: patch6264.diff added

comment:9 Changed 11 years ago by dylan

Owner: set to dylan
Status: newassigned

comment:10 Changed 11 years ago by dylan

Milestone: 1.21.1.1

comment:11 Changed 11 years ago by dylan

Resolution: fixed
Status: assignedclosed

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

comment:12 Changed 11 years ago by dylan

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

comment:13 Changed 11 years ago by dylan

Resolution: fixed
Status: closedreopened

comment:14 Changed 11 years ago by dylan

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

comment:15 in reply to:  14 Changed 11 years ago by Sam Foster

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.

comment:16 Changed 11 years ago by Adam Peller

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

comment:17 Changed 11 years ago by bill

Milestone: 1.1.11.3

comment:18 Changed 11 years ago by Sam Foster

Milestone: 1.31.2
Owner: changed from dylan to Sam Foster
Status: reopenednew

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

comment:19 Changed 11 years ago by Sam Foster

Status: newassigned

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

comment:20 Changed 11 years ago by bill

Resolution: fixed
Status: assignedclosed

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

Changed 11 years ago by Sam Foster

Attachment: 6264_mozFocusInner.patch added

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

comment:21 Changed 11 years ago by Sam Foster

Resolution: fixed
Status: closedreopened

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.

comment:22 Changed 11 years ago by dante

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

comment:23 Changed 11 years ago by dante

Resolution: fixed
Status: reopenedclosed
Note: See TracTickets for help on using tickets.