Opened 10 years ago

Closed 7 years ago

#8827 closed bug (fixed)

Button: Pixel positioning for button widget with radio buttons is incorrect

Reported by: ezyang Owned by:
Priority: minor Milestone: 1.12.0
Component: ui.button Version: 1.9.1
Keywords: Cc:
Blocked by: Blocking:

Description

The default theme for 'button' incorrectly uses negative margins in 'em'. You can see the problem by inspecting the demo on http://jqueryui.com/button/#radio on Chrome (I'm on Version 22.0.1229.94 Ubuntu 12.10 (161065) but I imagine the problem is persistent over many versions)

The first button is not properly offset with the second button; when #1 is selected and #2 highlighted, the border is 2px wide. The second button is properly offset with the third, but its right border is covered by the third button, so it doesn't darken even when selected.

Because the borders of these buttons are measured in pixels, any negative margins should be in pixels as well. This also prevents distortions when the font size of a document is different.

Change History (9)

comment:1 Changed 10 years ago by tj.vantoll

Status: newopen
Summary: Pixel positioning on default theme for button widget with radio buttons is incorrectButton: Pixel positioning on default theme for button widget with radio buttons is incorrect

Verified. Made an example on jsFiddle so it's easier to play with - http://jsfiddle.net/tj_vantoll/9nD2v/.

comment:2 Changed 10 years ago by ezyang

Funnily enough, it renders fine for me on the jsFiddle :-) (Also, it would be good to set the z-index of the focused element higher than the rest, so the border extends over the entire of the button and isn't clipped off by a button on the right.)

comment:3 Changed 10 years ago by tj.vantoll

Milestone: 1.10.0none

comment:4 Changed 10 years ago by tj.vantoll

#9398 is a duplicate of this ticket.

comment:5 Changed 10 years ago by tj.vantoll

Summary: Button: Pixel positioning on default theme for button widget with radio buttons is incorrectButton: Pixel positioning for button widget with radio buttons is incorrect

It's not just for the default theme. This is actually much easier to visually see with dark themes. See this test case from @Dinosaurs in #9398: http://jsfiddle.net/jakecigar/2wCFx/.

comment:6 Changed 10 years ago by kbwood

This CSS fixes the example given above:

.ui-button.ui-state-active { z-index: 10; }

comment:7 in reply to:  6 Changed 10 years ago by tj.vantoll

Replying to kbwood:

This CSS fixes the example given above:

.ui-button.ui-state-active { z-index: 10; }

I like that fix. It also needs to be applied to .ui-button.ui-state-hover to cover the OP's use case, and the z-index only has to be 1 to trump the sibling buttons.

.ui-buttonset .ui-button.ui-state-active, .ui-buttonset .ui-button.ui-state-hover {
    z-index: 1;
}

Seems to work. @kbwood, would you be interested in creating a pull request for this?

comment:8 Changed 10 years ago by tj.vantoll

Actually after taking a second look the z-index does solve the border overlap, but one of the original problems... "when #1 is selected and #2 highlighted, the border is 2px wide", isn't solved by this.

I think we need to define the negative margins in pixels as the OP suggested.

comment:9 Changed 7 years ago by Scott González

Milestone: none1.12.0
Resolution: fixed
Status: openclosed

Controlgroup (the replacement for buttonset) no longer uses "overlapping" borders, so this is effectively fixed in master.

Note: See TracTickets for help on using tickets.