The Storage Inspector should have a button for toggling the display of the sidebar like the one the Network panel has. Sebastian

I figure this should be a P2 as we need consistency across our tools.

Filter on Brobdingnagian.

Comment on attachment 8847087 [details] Bug 1264585 - Add sidebar toggle button to storage panel https://reviewboard.mozilla.org/r/120140/#review121982 I don't think I need to re-review this, but I do have a few comments that I'd like addressed. ::: devtools/client/storage/test/browser.ini:57 (Diff revision 1) > [browser_storage_overflow.js] > [browser_storage_search.js] > [browser_storage_search_keyboard_trap.js] > [browser_storage_sessionstorage_edit.js] > [browser_storage_sidebar.js] > +[browser_storage_sidebar_toggle.js] Can you also add a second test that just checks the visibily of the button itself? If something is selected then it should be shown, otherwise it should be hidden. ::: devtools/client/storage/test/browser_storage_sidebar_toggle.js:8 (Diff revision 1) > +// <id of the table item to click> or <id array for tree item to select> or > +// null to press Escape, > +// <do we wait for the async "sidebar-updated" event>, > +// <is the sidebar open> This comment seems out of date with the actual testCase content. ::: devtools/client/storage/ui.js:227 (Diff revision 1) > this._tablePopupDeleteAll.removeEventListener("command", this.onRemoveAll); > }, > > + setupToolbar: function () { > + this.searchBox = this._panelDoc.getElementById("storage-searchbox"); > + this.filterItems = this.filterItems.bind(this); Please move all the function bindings to the constructor. That's where you already bind `onPaneToggleButtonClicked`, so you might as well do them all up there.

Pushed by mratcliffe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d13bb1ff158a Add sidebar toggle button to storage panel r=pbro

The screenshots and/or description at https://developer.mozilla.org/en-US/docs/Tools/Storage_Inspector should be updated to include this change. Sebastian

This bug was about "adding sidebar toggle button to Storage Inspector" and I have seen the feature being implemented in latest Nightly! This bug's fix is verified with latest Nightly on WIndows 7, 64 Bit! Build ID : 20170502030211 User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0 [bugday-20170503]

I just realized that the UX is different between the button in the Network Monitor and the Storage Inspector. In the Network Monitor, if you close the sidebar, the selected entry gets deselected and so the sidebar gets opened again once you select another entry. In the Storage Inspector, if you close the sidebar, the previously selected entry stays selected and the sidebar stays closed when you select another entry. This is the same behavior as in the DOM Inspector, while the main UI of the Storage Inspector is rather similar to the one of the Network Monitor. I don't have a strong opinion on whether this needs to be changed, but it might be unexpected for users. Mike, I let you decide whether one of them should be changed to match the other, and if so, which one to change. Sebastian

In the Storage Inspector if you click the sidebar toggle button you can select rows without the sidebar interfering. The sidebar reappears only if the sidebar toggle button is clicked. We chose to do it this way because, unlike other tools, the Storage Inspector table is editable and the sidebar can quickly become irritating when it comes to editing rows of text. Personally, I would say that the network monitor should follow the same pattern as people don't always want to see the sidebar... toggling the sidebar off should leave it off until it is toggled on again. @Honza: We should make these consistent... are you aware of a bug to change the behaviour?

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #12) > @Honza: We should make these consistent... are you aware of a bug to change > the behaviour? I agree (and I was suggesting to change that in the past). I think that the reason why the Net monitor panel is supporting this auto-sidebar-open feature is consistency with Chrome. Anyway, there is no such bug reported AFAIK. Feel free to create one. Honza

(In reply to Jan Honza Odvarko [:Honza] from comment #13) > (In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #12) > > @Honza: We should make these consistent... are you aware of a bug to change > > the behaviour? > I agree (and I was suggesting to change that in the past). I think that the > reason why the Net monitor panel is supporting this auto-sidebar-open > feature is consistency with Chrome. Could be. Though the difference to Chrome is that it doesn't have a sidebar toggle button. > Anyway, there is no such bug reported AFAIK. Feel free to create one. Filed as bug 1361989. Sebastian