-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fixes multi inline select menu issues #8037
Conversation
@@ -279,6 +279,7 @@ div.ui-mobile-viewport { | |||
padding: 0; | |||
} | |||
.ui-btn-inline { | |||
max-width:100% ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be part of the inline rule instead the ui-select
rule I don't think we should restrict the width of inline buttons in general. @jaspermdegroot @sfrisk thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arschmitz Firstly i was thinking of adding a new class to selectmenu which restricts its width when it goes out of the page. This class would only be added to element having .ui-select class and only when it goes out of the page. Do you think this idea is better..??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Let's fix this in the selectmenu CSS.
I think we should also look at old (closed) tickets because I know we have been dealing with select width issues in the past and don't want regressions. Might be good to add a unit test for this.
We should do some testing with max-width: 100%;
for the selectmenu button in the ui-1-12 branch after my CSS update (no PR yet) landed in there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaspermdegroot done 👍
unit test written..please check if its correct..
cd81556
to
c21d779
Compare
test( "Custom select Multiple Inline width is adjusted correctly", function() { | ||
var selectMenu = $( "#width-test" ); | ||
var parent = $( "#width-test" ).parent(); | ||
var widths = [100, 200, 300, 400, 500, 600, 700, 800, 900, 1000]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing spaces in side brackets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testing this many widths seems like overkill
200938b
to
d06321f
Compare
@@ -66,6 +66,10 @@ | |||
.ui-select .ui-btn-icon-right .ui-li-count { | |||
right: 3.2em; | |||
} | |||
/*Fixes #7918 : Count indicator position must be different for mini version*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't reference issues in comments. Blank line before comments. Space before first word and after last word.
result = 0; | ||
|
||
$.each( widths , function( index, width ){ | ||
parent.css( {"width": width+"px"} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spaces inside brackets
Fixes #7918
Fixes #8038