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
Status: | new → open |
---|---|
Summary: | Pixel positioning on default theme for button widget with radio buttons is incorrect → Button: Pixel positioning on default theme for button widget with radio buttons is incorrect |
comment:2 Changed 10 years ago by
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
Milestone: | 1.10.0 → none |
---|
comment:5 Changed 10 years ago by
Summary: | Button: Pixel positioning on default theme for button widget with radio buttons is incorrect → Button: 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 follow-up: 7 Changed 10 years ago by
This CSS fixes the example given above:
.ui-button.ui-state-active { z-index: 10; }
comment:7 Changed 10 years ago by
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
comment:9 Changed 7 years ago by
Milestone: | none → 1.12.0 |
---|---|
Resolution: | → fixed |
Status: | open → closed |
Controlgroup (the replacement for buttonset) no longer uses "overlapping" borders, so this is effectively fixed in master.
Verified. Made an example on jsFiddle so it's easier to play with - http://jsfiddle.net/tj_vantoll/9nD2v/.