1209641 - Remove dropdown view and use sidebars always.

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[Jen Fong-Adwent [:ednapiranha]]

Remove old tests, conditionals to check the sidebar pref

Comment 1

[Alfred Kayser]

Also change the toolbardropdown button into "context/simulator" status indicator as it is no longer opening a dropdown/panel.
And remove the "app" toolbarbutton.
Possibly combine the play/pause/debug buttons, and the above status indicator in the menubar, freeing up space for the app/debug panes.

Comment 2

[Jen Fong-Adwent [:ednapiranha]]

(In reply to Alfred Kayser from comment #1)
> Also change the toolbardropdown button into "context/simulator" status
> indicator as it is no longer opening a dropdown/panel.
> And remove the "app" toolbarbutton.
> Possibly combine the play/pause/debug buttons, and the above status
> indicator in the menubar, freeing up space for the app/debug panes.

We can just file new bugs for these changes rather than bundle it into this particular bug.

Comment 3

[Jen Fong-Adwent [:ednapiranha]]

Attachment: Bug1209641.patch

I *think* I removed most of the css, xul and js that was specific to the dropdown layout but could be missing things - if you see anything obvious, let me know so I can update.

I also decided to keep the [sidebar-displayed] rule in webide.css for the situation in which we might decide to do a toggle view later.

Also try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3acc9bc4795e

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8669155 [details] [diff] [review]
Bug1209641.patch

Review of attachment 8669155 [details] [diff] [review]:
-----------------------------------------------------------------

Works well, it's quite close!  Just a few more CSS cleanups it seems.

For the tests, did you just copy the sidebars ones on top of the old files?  I did not review them specifically yet, I wasn't sure how closely to look at them.

::: devtools/client/webide/content/webide.js
@@ +68,5 @@
>  
>      this.appManagerUpdate = this.appManagerUpdate.bind(this);
>      AppManager.on("app-manager-update", this.appManagerUpdate);
>  
> +    ProjectPanel.toggleSidebar();

Change this to `Cmds.showProjectPanel` to parallel the line below.

We could get rid of the panel commands entirely... but we may add a real toggle, so they would make sense to have if we do that.

@@ -1067,5 @@
> -    if (panel && panel.id == "deck-panel-details" &&
> -        AppManager.selectedProject &&
> -        AppManager.selectedProject.type != "packaged" &&
> -        this.toolboxIframe) {
> -      nbox.setAttribute("toolboxfullscreen", "true");

You should also remove the "toolboxfullscreen" styles from the CSS.

@@ -1139,5 @@
> -    if (!projectList.sidebarsEnabled && AppManager.connected) {
> -      projectList.refreshTabs();
> -    }
> -
> -    return promise.resolve();

A test yields on this function, so I think we still need to return a promise here (or in ProjectPanel.toggleSidebar).

::: devtools/client/webide/content/webide.xul
@@ -151,5 @@
>  
> -  <popupset>
> -
> -    <!-- App panel -->
> -    <panel id="project-panel" type="arrow" position="bottomcenter topleft" consumeoutsideclicks="true" animate="false">

There is some CSS specific to panels, should be able to remove it now.

@@ -153,5 @@
> -
> -    <!-- App panel -->
> -    <panel id="project-panel" type="arrow" position="bottomcenter topleft" consumeoutsideclicks="true" animate="false">
> -      <vbox flex="1" id="project-panel-box">
> -        <toolbarbutton class="panel-item project-panel-item-newapp" command="cmd_newApp"/>

Lots of CSS left for panel items, those are now in the list-specific CSS files, so can be removed from webide.css.

@@ -180,5 @@
> -        <toolbarbutton class="panel-item" label="&runtimePanel_installsimulator;" id="runtime-panel-installsimulator" command="cmd_showAddons"/>
> -        <label class="panel-header">&runtimePanel_other;</label>
> -        <vbox id="runtime-panel-other"></vbox>
> -        <vbox flex="1" id="runtime-actions">
> -          <toolbarbutton class="panel-item" id="runtime-details" command="cmd_showRuntimeDetails"/>

Several CSS blocks for runtime toolbarbuttons can be removed.

::: devtools/client/webide/modules/project-list.js
@@ +124,5 @@
>     *   icon: String       path of the project icon
>     * }
>     */
>    _renderProjectItem: function(opts) {
> +    if (this._doc !== this._parentWindow.document) {

I believe this is always true, so we could remove it.

::: devtools/client/webide/themes/webide.css
@@ +58,5 @@
>    width: 12px;
>    height: 7px;
>  }
>  
>  .panel-button:hover > .panel-button-anchor {

I believe you can remove this and the block above, since all the anchors are gone now.

Comment 5

[Jen Fong-Adwent [:ednapiranha]]

Attachment: Bug1209641.patch

Updated with the changes you suggested.

As for tests, they were copied over from the old sidebars/ directory with the exception of test_simulators.html, which I had to change in order for them to pass.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=78130a02a370

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8670051 [details] [diff] [review]
Bug1209641.patch

Review of attachment 8670051 [details] [diff] [review]:
-----------------------------------------------------------------

A few CSS tweaks left, but looks good overall!

::: devtools/client/webide/themes/webide.css
@@ -52,5 @@
>  }
>  
> -/* Panel buttons */
> -
> -.panel-button {

Do we need this bit still?  We still have "runtime-panel-button" left.  Check if the styles have any visual effect.

@@ +93,5 @@
>  #action-button-debug[active="true"] { -moz-image-region: rect(100px,300px,200px,200px) }
>  
>  /* Panels */
>  
>  panel > vbox {

There is no panel, can we remove it?

@@ +134,4 @@
>    display: block;
>  }
>  
>  .panel-item {

There is no .panel-item in webide.xul, can we remove it?

Comment 7

[Jen Fong-Adwent [:ednapiranha]]

Attachment: Bug1209641.patch

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/fd6d8faa4621

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fd6d8faa4621