Closed Bug 1525821 Opened 5 years ago Closed 5 years ago

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

Categories

(DevTools :: Console, defect, P2)

67 Branch
defect

Tracking

(firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: tanhengyeow, Assigned: b17071, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug)

Attachments

(3 files, 2 obsolete files)

Attached image 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.

Attached image 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."

Blocks: 1523842
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2

Hello ithompson4,

Would you be interested to work on this bug?

Mentor: nchevobbe
Flags: needinfo?(ithompson4)
Keywords: good-first-bug

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

Flags: needinfo?(ithompson4)

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 :)

OK, Thank you.

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.

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.

Assignee: nobody → b17071
Status: NEW → ASSIGNED

(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

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.

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

Attachment #9044049 - Attachment description: Bug 1525821 -Implement_cancel_button r=nchevobbe@mozilla.com! → Bug 1525821 -Add clear button in the filter input; r=nchevobbe@mozilla.com!

style of filter bar changed according to the requirement

Attachment #9045805 - Attachment is obsolete: true

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

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

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 :) )

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

Hi Nicolas,

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

thanks
Aaditya

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

Attachment #9047198 - Attachment description: Add a clear button in the filter input, like we already do in Netmonitor → Bug 1525821 - Add a clear button in the console filter input. r=nchevobbe.
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39380f2678a3
Add a clear button in the console filter input. r=nchevobbe.

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

Flags: needinfo?(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 :)

Flags: needinfo?(nchevobbe)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Attachment #9044049 - Attachment is obsolete: true

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.

Flags: needinfo?(nchevobbe)

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

Flags: needinfo?(nchevobbe)
Depends on: 1533277

(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

thanks Tim!

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: