1525821 - Add a 'X' button for the Console panel's filter pane

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Description

[Hengyeow (:tanhengyeow)]

Attachment: Console-filter.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36

Steps to reproduce:

In the netmonitor panel, when one is typing in the filter pane, there is a X button present to clear the text.

Actual results:

The console filter pane doesn't have this 'X' button present when typing in the filter pane.

Expected results:

The 'X' button should be present in the Console panel's filter pane as well.

Comment 1

[Hengyeow (:tanhengyeow)]

Attachment: Netmonitor-filter.png

"In the netmonitor panel, when one is typing in the filter pane, there is a X button present to clear the text."

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Hello ithompson4,

Would you be interested to work on this bug?

Comment 3

[ithompson4]

Yes I am very interested. Would mid March be OK as a target for a pull request?

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

I think we want to have this done for the 67 release, which will happen at this time, so we would be a bit short in time.
I'll try to find you another bug that don't have such a deadline :)

Comment 5

[ithompson4]

OK, Thank you.

Comment 6

[Aaditya Arora]

Hi Nicolas,
I have looked into the cause of this issue. in the case of netmonitor, filter input is using SearchBox for input which has cancel button implemented whereas in webconsole its using simple dom's input.

I would like to work on this bug.

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Hello Aaditya, thanks for wanting to help us!
I'll assign the bug to you.

I suggest you to read http://docs.firefox-dev.tools/getting-started/ to setup the work environment.
Don't hesitate to ask questions, either here or on our Slack.

Comment 8

[Aaditya Arora]

(In reply to Nicolas Chevobbe from comment #7)

I suggest you to read http://docs.firefox-dev.tools/getting-started/ to setup the work environment.

thanks, I have already setup the working environment.

Don't hesitate to ask questions, either here or on our Slack.

Yaa, sure. thanks

Comment 10

[:Harald Kirschner :digitarald]

Bug 1525821 adds (on top the clear button) an interesting aspect of making sure the filtered state is clear; which we might be consider here as well.

As simple solution the filter icon could turn blue (similar to other enabled button states across devtools). We can bounce it off UX after implementation.

Comment 11

[Aaditya Arora]

Attachment: Bug 1525821 -Add clear button in the filter input; r=[e-mail]!

implmented cancel button for filters using the same ui as of Search Box

Comment 12

[Aaditya Arora]

Attachment: Address revision issues : change the style

style of filter bar changed according to the requirement

Comment 13

[Aaditya Arora]

Please ignore this attachment, I made some mistakes. I'll submit the fresh patch for revision again.

Comment 14

[Aaditya Arora]

Hi Nicolas,
I am new to Phabricator, I have created a commit using hg commit and added some specific files in this commit. but when I did arc diff. It added all files for review (i.e., files which were not added in commit using hg add). Why?

thanks

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

Hello Aaditya, first, thanks for working on this :)
In regards to arc diff, I don't use it so I can't really tell you.
I'm using mozilla-conduit/review and I'm quite pleased with it.
Let me know if you have any trouble with this. (also, next time, try to set the needinfo field here so I'm less likely to miss one of your message :) )

Comment 16

[Aaditya Arora]

Attachment: Bug 1525821 - Add a clear button in the console filter input. r=nchevobbe.

initially there was no clear button in filter input box , now added a clear button for it

Comment 17

[Aaditya Arora]

Hi Nicolas,

Created final patch with changed styles and included tests.
please review this patch

thanks
Aaditya

Comment 18

[Aaditya Arora]

Hi Nicolas,

Committed all changes that is required.
I have a question, Is updating a given patch sends u a notification email about the changes` ? or need to comment here referencing the update ?

thanks

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39380f2678a3
Add a clear button in the console filter input. r=nchevobbe.

Comment 20

[Aaditya Arora]

Hi Nicolas,

Just for the sake of information, can we close this bug now ? or it still requires some more process to be done ?

thanks
Aaditya

Comment 21

[Nicolas Chevobbe [:nchevobbe]]

Hello Aaditya, so the bug isn't resolved yet. It was pushed to the autoland branch, were additional test are going to be run. Then, if everything is okay, it will be merged into mozilla-central, and that's when a code sheriff will set the bug as RESOLVED FIXED. It can then take a day until it hits Firefox Nightly.
So here, I guess the bug will be resolved later today, or tomorrow, and will be in Nightly by Monday for sure :)

Comment 22

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/39380f2678a3

Comment 23

[Tim Nguyen :ntim]

Nicolas, was there a good reason not to use the shared SearchBox component that provides this functionality ?

It's already used in the network monitor and the DOM panel.

Comment 24

[Nicolas Chevobbe [:nchevobbe]]

No, I wasn't aware of this component :/
Should we file a bug to switch to it?

Comment 25

[Tim Nguyen :ntim]

(In reply to Nicolas Chevobbe from comment #24)

No, I wasn't aware of this component :/
Should we file a bug to switch to it?

Filed bug 1533277

Comment 26

[Nicolas Chevobbe [:nchevobbe]]

thanks Tim!