1395825 - Improve filter input

Status: RESOLVED DUPLICATE of bug 1525821

(DevTools :: Console, enhancement, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

This comes from the webcompat team https://github.com/orgs/devtools-html/projects/2#card-4434699

Add a simple clear button on the console's text-filter, and more prominent
visual indication when it's active (both `:focus` and `:not(:empty)` style should be considered).

Comment 1

[[:Towkir] Ahmed]

Hey Nicolas, 
I am interested on this one.
The link you provided on comment 0 returns 404. No idea why, there shouldn't be some access permission, or is there ?
I think the mentioned issue (on the link) contains more details about this bug.
Can you please check / provide a way to access / another link for that so that I can have a look ?
Thanks

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Hello Ahmed, 
There is not much on the card either : 

> Add clear button on text-filter input
> More prominent visual indication when active (applies to other such filters too, like in the inspector)
> https://groups.google.com/forum/#!topic/mozilla.compatibility/VlloOE

Do you need further information ?

Comment 3

[[:Towkir] Ahmed]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
> https://groups.google.com/forum/#!topic/mozilla.compatibility/VlloOE
This link says "The topic ID is invalid"

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(In reply to [:Towkir] Ahmed from comment #3)
> (In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
> > https://groups.google.com/forum/#!topic/mozilla.compatibility/VlloOE
> This link says "The topic ID is invalid"

Oh, must have been deleted.
Anyway, it should really be about adding proper :focus styling, as well as a "clear" button (like in the inspector Rules panel, it-appears as soon as the filter is not empty)

Comment 6

[[:Towkir] Ahmed]

I hope I understand what to be done, will submit a patch and assign this to me.
Thanks

Comment 7

[[:Towkir] Ahmed]

Attachment: web-console-filterbar.png

Hi Victoria,
Have a look at the attached image, it represents the focus styling and also the clear button, it should appear when the bar is not empty of course, made it visible just for having a look for now.

and the border radius is 20px on mac, and 2px on other platforms of course (like any other filter input or search input in devtools)

Thanks

Comment 8

[[:Towkir] Ahmed]

Also, do you think we should go for a 'thin' clear button for these filterinputs ?

Comment 9

[[:Towkir] Ahmed]

Attachment: Bug 1395825 - Improved filter input with some foucs styling and added a clear button;

Review commit: https://reviewboard.mozilla.org/r/206920/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/206920/

Comment 10

[[:Towkir] Ahmed]

Comment on attachment 8936163 [details]
Bug 1395825 - Improved filter input with some foucs styling and added a clear button;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/206920/diff/1-2/

Comment 11

[Victoria Wang [:victoria]]

Thanks Ahmed - this clear button look great!

The focus ring is actually being worked on in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1421395 Hopefully :nchevobbe or :gl can help resolve/merge the work being done.

One thing that needs to be fixed: for these long input bars in DevTools, there should be no border radius (even on Mac). I will revisit the clear button later so that it looks better in a rectangular search field.

Comment 12

[[:Towkir] Ahmed]

Usually the rectangular ones have a radius of 2px which is very minor, still makes this look good.
not even that 2px radius ?

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8936163 [details]
Bug 1395825 - Improved filter input with some foucs styling and added a clear button;

https://reviewboard.mozilla.org/r/206920/#review213872

Thanks Ahmed ! The JS code looks good, and the CSS should change to match what Victoria said :)
Also, one mocha test is failing because of the changes in the Filter component. So we need to fix it.
You can run the mocha test by doing : cd devtools/client/webconsole && yarn install && yarn test

::: devtools/client/themes/webconsole.css:853
(Diff revision 2)
> +  margin: 1px 3px;
> +  border: 1px solid;
> +  border-radius: 2px;
> +  padding: 4px 10px;
> +  border-color: var(--theme-splitter-color);
> +  font: message-box;

not sure we should have any of this

::: devtools/client/themes/webconsole.css:861
(Diff revision 2)
> +:root[platform="mac"] .webconsole-filter-wrapper .devtools-plaininput {
> +  border-radius: 20px;
> +}

let's remove this

Comment 14

[[:Towkir] Ahmed]

Comment on attachment 8936163 [details]
Bug 1395825 - Improved filter input with some foucs styling and added a clear button;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/206920/diff/2-3/

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8936163 [details]
Bug 1395825 - Improved filter input with some foucs styling and added a clear button;

https://reviewboard.mozilla.org/r/206920/#review213946

r- since there are failures on try at the moment

::: commit-message-3a33e:1
(Diff revision 3)
> +Bug 1395825 - Improved filter input with some foucs styling and added a clear button; r?nchevobbe

s/foucs/focus

::: devtools/client/themes/webconsole.css:863
(Diff revision 3)
> +}
> +
> +.webconsole-filter-wrapper .devtools-plaininput:focus {
> +  border-color: var(--theme-focus-border-color-textbox);
> +  box-shadow: var(--theme-focus-box-shadow-textbox);
> +  transition: all 0.2s ease-in-out;

I think we could use one of the recommanded bezier curve in photon design system https://design.firefox.com/photon/motion/duration-and-easing.html

Luckily, it's already in a CSS variable to consume in the devtools https://searchfox.org/mozilla-central/rev/b1e0ae2573e88391a7ed92407568be77994c9317/devtools/client/themes/variables.css#286

::: devtools/client/themes/webconsole.css:869
(Diff revision 3)
> +  outline: none;
> +}
> +
> +.webconsole-filter-wrapper .devtools-searchinput-clear {
> +  display: inline-block;
> +  top: 4.5px;

setting a top property feels a bit weird. 

Since we can't have this button placed using flexbox properties, what do you think of using a simple grid on webconsole-filter-wrapper ? 

it would be a 1 row, 2 column grid.
The filter would be placed on both cells (grid-rows: 1 / -1), and the button only on the last one (grid-rows: -1 / -2)

So this way, the button overlap the input, in the way we want. And we can use align-self to center it vertically.

Comment 16

[Victoria Wang [:victoria]]

Re: border radius, I've been looking into this (other places in Firefox do use 2px, 3px, or 4px radii for input elements). I think for this situation in DevTools, it would be best to go with 0 radius for now since it's a full-width/height row input - more similar to Debugger/Style Editor's boxy styling than the other types of inputs in the browser. Thanks!

Comment 17

[Victoria Wang [:victoria]]

Comment on attachment 8936133 [details]
web-console-filterbar.png

Already wrote my comments in the bug so clearing this review. (Just need the border radius to be 0)

Comment 18

[Victoria Wang [:victoria]]

Comment on attachment 8936133 [details]
web-console-filterbar.png

Sorry, actually clearing the flag now

Comment 19

[Gabriel Luong [:gl] (ΦωΦ)]

If we are adding a clear button, please take a look at the CSS used in the Filter Styles input in the Rules panel. We already have common CSS classes that implements the clear button and positions/shows it when necessary. I also wonder if we intended to keep the clear button for the photon design.

Comment 20

[Nicolas Chevobbe [:nchevobbe]]

The clear button is a request from the webcompat team.

Comment 21

[Victoria Wang [:victoria]]

I think I was originally going to design a new clear button for the new styling and now I think that was not necessary -  We can just match the look of the clear button in the about:preferences search field. (Netmonitor clear button needs positioning polish as well - ideally they both match about:preferences.)

Also, now that the Netmonitor has the white background on the filter input row, it would be great if Console matched it.

Comment 22

[Victoria Wang [:victoria]]

Another thing: I think we should consider allowing Esc to clear filter input, as is the case in Rules filter, Debugger search, and Chrome's console filter. I do worry people are using Esc to navigate to the console input. In that case we'd need some other way to jump to the input.

Comment 23

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Victoria Wang [:victoria] from comment #22)
> Another thing: I think we should consider allowing Esc to clear filter
> input, as is the case in Rules filter, Debugger search, and Chrome's console
> filter. I do worry people are using Esc to navigate to the console input. In
> that case we'd need some other way to jump to the input.

Do you mean when the filter input is focused ? Or simply when the console is visible and the filter input non-empty ?
Also, this not seems consistent everywhere (the search filter in the markup view doesn't respond to Esc for example). 
So if we are going to do this, it would be nice to have a meta bug for it and file individual bug on each filter input that would need it.

Comment 24

[Victoria Wang [:victoria]]

I think the ESC clear behavior is safe for when the filter is focused. (Clearing any time the input is non-empty seems like it would clash with the ESC for Console behavior in other panels).

Yes, you're right about how it's inconsistent in many panels - I filed the meta bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1473745

Comment 25

[:Harald Kirschner :digitarald]

Work continues as part of the console toolbar redesign.