Closed
Bug 455113
Opened 16 years ago
Closed 16 years ago
Tabbar and control-bar should not show/hide with the same drag event
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
fennec1.0a1
People
(Reporter: romaxa, Unassigned)
Details
Attachments
(2 files, 1 obsolete file)
3.40 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
1.10 KB,
patch
|
Details | Diff | Splinter Review |
Steps: 1. Drag from left -> right 2. Tab Bar is visible 3. Drag from right -> left EXPECTED OUTCOME: Tab Bar disappear, only layout visible ACTUAL OUTCOME: Tab Bar disappear, Control bar appears on the right side.... I think this is not very good from usability point of view... Sometime it is very hard to make visible only layout on screen (without any bars...)
Updated•16 years ago
|
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 2•16 years ago
|
||
I think I have to reopen this bug. I still very often able to open Tabbar while closing "Control bar" and vice versa.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Reporter | ||
Comment 3•16 years ago
|
||
Attachment #341724 -
Flags: review?(mark.finkle)
Comment 4•16 years ago
|
||
Comment on attachment 341724 [details] [diff] [review] Proposed fix First pass review: Looks like you wanted to use draggingData.downUiMode to hold the original "mode", but then added this.mouseDownMode. Let's switch to use dragginData.uiMode instead of this.mouseDownMode. I still need to apply the patch and test.
Reporter | ||
Comment 5•16 years ago
|
||
Attachment #341724 -
Attachment is obsolete: true
Attachment #341744 -
Flags: review?(mark.finkle)
Attachment #341724 -
Flags: review?(mark.finkle)
Comment 6•16 years ago
|
||
Comment on attachment 341744 [details] [diff] [review] Updated from comment patch >diff -r 9f1ea2560899 chrome/content/browser-ui.js >--- a/chrome/content/browser-ui.js Sat Oct 04 03:52:16 2008 -0400 >+++ b/chrome/content/browser-ui.js Sat Oct 04 16:17:40 2008 -0400 >@@ -208,17 +208,18 @@ var BrowserUI = { > dragging : false, > startX : 0, > startY : 0, > dragX : 0, > dragY : 0, > lastX : 0, > lastY : 0, > sTop : 0, >- sLeft : 0 >+ sLeft : 0, >+ uiMode : UIMODE_NONE > }, > > _scrollToolbar : function bui_scrollToolbar(aEvent) { > var [scrollWidth, ] = Browser.content._contentAreaDimensions; > var [viewportW, ] = Browser.content._effectiveViewportDimensions; > > var pannedUI = false; > >@@ -282,16 +283,30 @@ var BrowserUI = { > let contentW = gContentBox.boxObject.width; > > // Limit the panning > if (newLeft > 0) > newLeft = 0; > else if (newLeft < -(tabbarW + sidebarW)) > newLeft = -(tabbarW + sidebarW); > >+ // Set the UI mode based on where we ended up >+ if (newLeft > -(tabbarW - tabbarW / 3) && newLeft <= 0) { >+ if (this._dragData.uiMode == UIMODE_CONTROLS) >+ return; >+ this.mode = UIMODE_TABS; >+ } >+ else if (newLeft >= -(tabbarW + sidebarW) && newLeft < -(tabbarW + sidebarW / 3)) { >+ if (this._dragData.uiMode == UIMODE_TABS) >+ return; >+ this.mode = UIMODE_CONTROLS; >+ } >+ else >+ this.mode = (gContentBox.style.marginTop == "0px" ? UIMODE_NONE : UIMODE_URLVIEW); >+ A couple of nits here: * this.mode should be reset, even if we return early * use braces on the "else" Here's a possible version that fixes both of those: // Set the UI mode based on where we ended up var noneMode = (gContentBox.style.marginTop == "0px" ? UIMODE_NONE : UIMODE_URLVIEW); if (newLeft > -(tabbarW - tabbarW / 3) && newLeft <= 0) { if (this._dragData.uiMode == UIMODE_CONTROLS) { this.mode = noneMode; return; } this.mode = UIMODE_TABS; } else if (newLeft >= -(tabbarW + sidebarW) && newLeft < -(tabbarW + sidebarW / 3)) { if (this._dragData.uiMode == UIMODE_TABS) { this.mode = noneMode; return; } this.mode = UIMODE_CONTROLS; } else { this.mode = noneMode; } r+ with those addressed > tabbar.left = newLeft; > > // Never let the toolbar pan off the screen > let newToolbarLeft = newLeft; > if (newToolbarLeft < 0) > newToolbarLeft = 0; > toolbar.left = newToolbarLeft; > >@@ -299,24 +314,16 @@ var BrowserUI = { > if (newLeft + tabbarW != 0) > toolbar.top = 0; > else > toolbar.top = this._contentTop - toolbar.boxObject.height; > > this._setContentPosition("left", newLeft + tabbarW); > sidebar.left = newLeft + tabbarW + contentW; > panelUI.left = newLeft + tabbarW + contentW + sidebarW; >- >- // Set the UI mode based on where we ended up >- if (newLeft > -(tabbarW - tabbarW / 3) && newLeft <= 0) >- this.mode = UIMODE_TABS; >- else if (newLeft >= -(tabbarW + sidebarW) && newLeft < -(tabbarW + sidebarW / 3)) >- this.mode = UIMODE_CONTROLS; >- else >- this.mode = (gContentBox.style.marginTop == "0px" ? UIMODE_NONE : UIMODE_URLVIEW); > > pannedUI = true; > } > } > > if (pannedUI) { > aEvent.stopPropagation(); > >@@ -798,19 +805,21 @@ var BrowserUI = { > case "mousedown": > this._dragData.dragging = true; > this._dragData.dragX = 0; > this._dragData.dragY = 0; > this._dragData.screenX = this._dragData.lastX = aEvent.screenX; > this._dragData.screenY = this._dragData.lastY = aEvent.screenY; > this._dragData.sTop = document.getElementById("toolbar-main").top; > this._dragData.sLeft = document.getElementById("tab-list-container").left; >+ this._dragData.uiMode = this.mode; > break; > case "mouseup": > this._dragData.dragging = false; >+ this._dragData.uiMode = UIMODE_NONE; > // Cause the UI to snap, if needed > this._showPanel(this.mode); > break; > case "mousemove": > this._scrollToolbar(aEvent); > break; > // Window size events > case "resize":
Attachment #341744 -
Flags: review?(mark.finkle) → review+
Updated•16 years ago
|
Flags: wanted-fennec1.0+
Target Milestone: --- → Fennec A1
Reporter | ||
Comment 7•16 years ago
|
||
Reporter | ||
Comment 8•16 years ago
|
||
Pushed in: http://hg.mozilla.org/mobile-browser/rev/09147f896864 http://hg.mozilla.org/mobile-browser/rev/aba146d25d5d
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•