Set larger label-filter default input click targets #1216
Conversation
|
what happens after you have filled out the first box? it goes back to only the width it needs? |
|
Yes. It only targets the initial default state
|
app/styles/_data-toolbar.less
Outdated
| .label-filter-key .selectize-input.not-full { | ||
| // Increase default input click target | ||
| width: 220px; | ||
| @media (min-width: @screen-xs) and (max-width: @screen-sm-max) { width: (@screen-xs - 100)} // 380px |
There was a problem hiding this comment.
I don't think you need the max-width here since you have a rule just below that sets the width for screen-md and larger
app/styles/_data-toolbar.less
Outdated
| // Increase default input click target | ||
| width: 220px; | ||
| @media (min-width: @screen-xs) and (max-width: @screen-sm-max) { width: (@screen-xs - 100)} // 380px | ||
| @media (min-width: @screen-md) { width: (@screen-xs + 20)} // 500px |
There was a problem hiding this comment.
I'm not sure how the width relates to screen-xs here. You set max-width: 600px; of the entire control above, so I'd think you would make width relative to that.
sg00dwin
force-pushed
the
default-filter-width
branch
from
8bdb6a7 to
e1242b2
Compare
|
@spadgett Updated pr
|
|
[test] |
app/styles/_data-toolbar.less
Outdated
| @@ -1,6 +1,12 @@ | |||
| /* Data toolbar | |||
| - Includes customizations for label filter input and active filter | |||
| ---------------------------------------------------------------------------- */ | |||
| @data-toolbar-filter-max-width: 772px; // 832 - 60; container - (left + right margin) at 992px viewport width | |||
There was a problem hiding this comment.
Thanks for making these variables.
Unclear where the 832 comes from. Would be better to calculate from variables like @grid-gutter-width and @screen-md etc where possible.
Same comment for some of the magic numbers below.
app/styles/_data-toolbar.less
Outdated
| width: @label-filter-default-input-xs; | ||
| @media (min-width: @screen-xs) { width: @label-filter-default-input-sm;} | ||
| @media (min-width: @screen-sm) { width: @label-filter-default-input-md;} | ||
| @media (min-width: @screen-md) { width: @label-filter-default-input-lg;} |
There was a problem hiding this comment.
Shouldn't these media queries use @screen-xs-min, @screen-sm-min and @screen-md-min ?
sg00dwin
force-pushed
the
default-filter-width
branch
3 times, most recently
from
876fad6 to
3a36dec
Compare
|
@spadgett Yep, updated the media query min variable. And for the
|
sg00dwin
force-pushed
the
default-filter-width
branch
from
3a36dec to
4054777
Compare
|
@spadgett did a clean install and updated pr |
sg00dwin
force-pushed
the
default-filter-width
branch
from
4054777 to
1bfa993
Compare
app/styles/_data-toolbar.less
Outdated
| @data-toolbar-filter-max-width: (@screen-md-min - (@sidebar-left-width-md + @middle-content-container-padding-lg + @middle-content-container-padding-lg)); | ||
| @label-filter-default-input-lg: (@label-filter-default-input-md + 190); // 730px | ||
| @label-filter-default-input-md: (@label-filter-default-input-sm + 140); // 540px | ||
| @label-filter-default-input-sm: (@label-filter-default-input-xs + 160); // 400px |
There was a problem hiding this comment.
@sg00dwin I don't think adding arbitrary amounts to the previous width variables here adds much value. I'd rather just set pixel sizes for each.
@label-filter-default-input-xs: 240px;
@label-filter-default-input-sm: 400px;
@label-filter-default-input-md: 540px;
@label-filter-default-input-lg: 730px;Or even inline the values if they're only used once.
There was a problem hiding this comment.
If you keep the variables names, maybe use label-filter-input-width-*? -width should be in there somewhere.
sg00dwin
force-pushed
the
default-filter-width
branch
from
1bfa993 to
01b07f9
Compare
|
@spadgett updated pr with rebase and clean/install |
statdard media query widths
sg00dwin
force-pushed
the
default-filter-width
branch
from
01b07f9 to
bcfdce1
Compare
|
Evaluated for origin web console test up to bcfdce1 |
|
@spadgett PTAL vendor.js diff is removed |
|
[merge] |
|
Evaluated for origin web console merge up to bcfdce1 |
|
Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1108/) (Base Commit: 3af61e9) |
|
Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1107/) (Base Commit: 3af61e9) |
Set reasonable widths on the default input