1523861 - Remove toggle filter button and always show the filter buttons toolbar

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

The toggle filter button is created in devtools/client/webconsole/components/FilterBar.js#261-266, so we can remove the whole dom.button block here.

It means we can remove the onClickFilterBarToggle method too (See FilterBar.js#105-107 and FilterBar.js#50).

Going further, we can remove the filterBarToggle action that was called in onClickFilterBarToggle (See devtools/client/webconsole/actions/ui.js#25-33,114), the type we were using for it (devtools/client/webconsole/constants.js#18) and its reducer case (devtools/client/webconsole/reducers/ui.js#40-41).

Then, we can always show the filter buttons toolbar by removing the if in devtools/client/webconsole/components/FilterBar.js#303-305

Then, some tests hide/show the toolbar (See this search result), so we should modify them to remove the parts where we hide the bar (since it can't be hidden anymore).

Comment 1

[Martin Koroknay]

Hello, I would be happy to take this on.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Hello Martin, thanks for wanting to help us!

I'm assigning the bug to you.

If it's your first time working on Firefox, I suggest you to read https://docs.firefox-dev.tools/getting-started/ to setup the work environment.

You can also come and talk on our Slack if you're blocked on something 🙂

Comment 3

[Martin Koroknay]

Attachment: Bug 1523861 - Remove toggle filter button and always show the filter buttons toolbar. r=nchevobbe

Removes the toggle filter button and makes the filter buttons toolbar permanently shown. At the same time updates some tests which relied on showing and hiding the filter buttons toolbar. Also amends an unrelated test which relied on the non-presence of the filter buttons toolbar.

Comment 4

[Martin Koroknay]

Hi, this is submitted for review now. Please note that I also had to update an unrelated test (devtools/client/webconsole/test/mochitest/browser_jsterm_no_input_and_tab_key_pressed.js) because it relied on the filter buttons not being there by default :)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19cdd41c33e6
Remove toggle filter button and always show the filter buttons toolbar. r=nchevobbe

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/19cdd41c33e6

Comment 7

[Rich Caloggero]

Why was the "toggle filter bar" button removed? When using a screen reader to examine devtools console output, the filter bar must be traversed before one gets to the start of the console output. The "toggle filter bar" was a welcome feature because it removed it from the DOM and allowed screen reader users to more quickly find the information they needed.

Comment 8

[James Teh [:Jamie]]

(In reply to Rich Caloggero from comment #7)

Why was the "toggle filter bar" button removed?

I'm curious about this as well. I guess there was some UX reason, but it's not documented in this bug.

When using a screen reader to examine devtools console output, the filter bar must be traversed before one gets to the start of the console output. The "toggle filter bar" was a welcome feature because it removed it from the DOM and allowed screen reader users to more quickly find the information they needed.

If there was a solid UX reason for removing this, perhaps we could add a role="main" to the output area (class webconsole-output) so screen reader users have a fast way to skip to the output. If that's the decided course here, we can file a new bug.

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Hello Rich, James.

The button was removed as we're redesigning the console toolbar (See Bug 1523842).
There will be 2 "modes": one where the filter buttons are always displayed (possibly in the same row as the filter input), and a "compact" one, where the filters will be put in a <select>, kind of what Chrome does.

This bug was the first step for this redesign, and I'm sorry it's making the developer experience worse for people who use screenreaders.

I will be more than happy to add a role="main" attribute to the output area, so I filed Bug 1530936 for that.