When ``too many'' tabs are added to the sidebar at least two problems arise: (1) The small height of the open tab can become unusable (2) New window + page load performance is hurt because it costs alot to paint each tab header To alleviate this problem we will limit the number of tabs in view. Users will need to navigate between tabs once they overflow the limit. We will add two navigation buttons at the bottom of the sidebar that are displayed only when the number of non-excluded tabs exceeds the number of tabs possible in view. The number of tabs in view will be a pref (potentially exposed in the UI via a future sidebar pref pane).

morse, please r. dveditz, please sr.

Comment on attachment 70014 [details] [diff] [review] Patch v1 r=morse

Comment on attachment 70014 [details] [diff] [review] Patch v1 >Index: xpfe/components/sidebar/resources/sidebarOverlay.xul >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/components/sidebar/resources/sidebarOverlay.xul,v >retrieving revision 1.81 >diff -u -6 -r1.81 sidebarOverlay.xul >--- xpfe/components/sidebar/resources/sidebarOverlay.xul 15 Feb 2002 12:33:30 -0000 1.81 >+++ xpfe/components/sidebar/resources/sidebarOverlay.xul 18 Feb 2002 01:23:52 -0000 >@@ -163,12 +163,20 @@ > <description>&sidebar.no-panels.state;</description> > <description>&sidebar.no-panels.add;</description> > <description>&sidebar.no-panels.hide;</description> > </vbox> > </vbox> > </vbox> >+ <hbox id="nav-buttons-box" hidden="true"> >+ <toolbarbutton flex="1" pack="center" class="sidebar-nav-button" >+ onclick="SidebarNavigate(-1);" >+ image="chrome://global/skin/arrow/arrow-up.gif"/> >+ <toolbarbutton flex="1" pack="center" class="sidebar-nav-button" >+ onclick="SidebarNavigate(1);" >+ image="chrome://global/skin/arrow/arrow-dn.gif"/> >+ </hbox> > <popupset id="contentAreaContextSet"/> > </vbox> > > <!-- Splitter on the right of sidebar --> > <splitter id="sidebar-splitter" collapse="before" persist="state hidden" > class="chromeclass-extrachrome sidebar-splitter" align="center" >Index: xpfe/components/sidebar/resources/sidebarOverlay.js >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/components/sidebar/resources/sidebarOverlay.js,v >retrieving revision 1.95 >diff -u -6 -r1.95 sidebarOverlay.js >--- xpfe/components/sidebar/resources/sidebarOverlay.js 7 Feb 2002 22:42:53 -0000 1.95 >+++ xpfe/components/sidebar/resources/sidebarOverlay.js 18 Feb 2002 01:23:53 -0000 >@@ -51,12 +51,16 @@ > } > > > // Uncomment for debug output > const SB_DEBUG = false; > >+// pref for limiting number of tabs in view >+// initialized in sidebar_overlay_init() >+var gNumTabsInViewPref; >+ > // The rdf service > const RDF_URI = '@mozilla.org/rdf/rdf-service;1'; > var RDF = Components.classes[RDF_URI].getService(); > RDF = RDF.QueryInterface(Components.interfaces.nsIRDFService); > > const NC = "http://home.netscape.com/NC-rdf#"; >@@ -81,12 +85,13 @@ > > function sbPanelList(container_id) > { > debug("sbPanelList("+container_id+")"); > this.node = document.getElementById(container_id); > this.childNodes = this.node.childNodes; >+ this.initialized = false; // set after first display of tabs > } > > sbPanelList.prototype.get_panel_from_id = > function (id) > { > debug("get_panel_from_id(" + id + ")"); >@@ -124,26 +129,26 @@ > sbPanelList.prototype.find_first = > function (panels) > { > debug("pick_default_panel: length=" + this.node.childNodes.length); > for (var ii = 2; ii < this.node.childNodes.length; ii += 2) { > var panel = this.get_panel_from_header_index(ii); >- if (!panel.is_excluded()) { >+ if (!panel.is_excluded() && panel.is_in_view()) { > return panel; > } > } > return null; > } > > sbPanelList.prototype.find_last = > function (panels) > { > debug("pick_default_panel: length=" + this.node.childNodes.length); > for (var ii=(this.node.childNodes.length - 1); ii >= 2; ii -= 2) { > var panel = this.get_panel_from_header_index(ii); >- if (!panel.is_excluded()) { >+ if (!panel.is_excluded() && panel.is_in_view()) { > return panel; > } > } > return null; > } > >@@ -157,12 +162,26 @@ > if (!panels.item(i).hidden) > return true; > } > return false; > } > >+sbPanelList.prototype.num_included_panels = >+function () >+{ >+ var count = 0; >+ var panels = this.node.childNodes; >+ for (var i = 2; i < panels.length; i += 2) >+ { >+ var curr = this.get_panel_from_header_index(i); >+ if (!curr.is_excluded()) >+ count++; >+ } >+ return count; >+} >+ > sbPanelList.prototype.select = > function (panel, force_reload) > { > if (!force_reload && panel.is_selected()) { > return; > } >@@ -194,23 +213,23 @@ > > // First, check the XUL for the "defaultpanel" attribute of "sidebar-box". > var sidebar_container = document.getElementById('sidebar-box'); > var content_default_id = sidebar_container.getAttribute('defaultpanel'); > if (content_default_id != '') { > var content = sidebarObj.panels.get_panel_from_id(content_default_id); >- if (content && !content.is_excluded()) { >+ if (content && !content.is_excluded() && content.is_in_view()) { > default_panel = content; > } > } > > // Second, try to use the panel persisted in 'last-selected-panel'. > if (!default_panel) { > var last_selected_id = this.node.getAttribute('last-selected-panel'); > if (last_selected_id != '') { > var last = sidebarObj.panels.get_panel_from_id(last_selected_id); >- if (last && !last.is_excluded()) { >+ if (last && !last.is_excluded() && last.is_in_view()) { > default_panel = last; > } > } > } > > // Finally, just use the last one in the list. >@@ -264,41 +283,83 @@ > if (sidebar_is_collapsed()) { > sidebarObj.collapsed = true; > } else { > sidebarObj.collapsed = false; > } > >+ if (sidebarObj.panels.num_included_panels() > gNumTabsInViewPref) >+ document.getElementById("nav-buttons-box").hidden = false; >+ else >+ document.getElementById("nav-buttons-box").hidden = true; >+ > var have_set_top = 0; > var have_set_after_selected = 0; > var is_after_selected = 0; > var last_header = 0; >+ var num_in_view = 0; >+ debug("this.initialized: " + this.initialized); > for (var ii=2; ii < this.node.childNodes.length; ii += 2) { > var header = this.node.childNodes.item(ii); > var content = this.node.childNodes.item(ii+1); > var id = header.getAttribute('id'); > var panel = new sbPanel(id, header, ii); >- if (panel.is_excluded()) { >- debug("item("+ii/2+") excluded"); >+ var excluded = panel.is_excluded(); >+ var in_view = false; >+ if (!this.initialized) >+ { >+ if (num_in_view < gNumTabsInViewPref) >+ in_view = true; >+ } >+ else >+ { >+ if (header.getAttribute("in-view") == "true") >+ in_view = true; >+ } >+ if (excluded || !in_view) >+ { >+ debug("item("+ii/2+") excluded: " + excluded + >+ " in view: " + in_view); > header.setAttribute('hidden','true'); > content.setAttribute('hidden','true'); >+ if (!in_view) >+ { >+ header.setAttribute("in-view", false); >+ header.removeAttribute("top-panel"); >+ header.removeAttribute("last-panel"); >+ } > } else { >- last_header = header; >+ // only set if in view >+ if (!this.initialized || (num_in_view < gNumTabsInViewPref)) >+ last_header = header; > header.removeAttribute('last-panel'); >- if (!have_set_top) { >+ // only set if in view >+ if (!have_set_top && >+ (!this.initialized || (header.getAttribute("in-view") == "true"))) >+ { > header.setAttribute('top-panel','true'); >- have_set_top = 1 >+ have_set_top = 1; > } else { > header.removeAttribute('top-panel'); > } > if (!have_set_after_selected && is_after_selected) { > header.setAttribute('first-panel-after-selected','true'); > have_set_after_selected = 1 > } else { > header.removeAttribute('first-panel-after-selected'); > } > header.removeAttribute('hidden'); >+ header.setAttribute("in-view", true); >+ num_in_view++; >+ >+ // selected tab is not in view so just select the last one >+ if (!is_after_selected && num_in_view == gNumTabsInViewPref && >+ selected_id != id) >+ { >+ selected_id = id; >+ this.node.setAttribute('last-selected-panel', id); >+ } > > // Pick sandboxed, or unsandboxed iframe > var iframe = panel.get_iframe(); > var load_state; > > if (selected_id == id) { >@@ -372,12 +433,14 @@ > // but some old profiles may have this persisted as hidden (50973). > this.node.removeAttribute('hidden'); > } else { > no_panels_iframe.removeAttribute('hidden'); > this.node.setAttribute('hidden','true'); > } >+ >+ this.initialized = true; > } > > > ////////////////////////////////////////////////////////////////////// > // sbPanel Class > // >@@ -452,12 +515,18 @@ > sbPanel.prototype.is_excluded = > function () > { > return sb_panel_is_excluded(this.get_header()); > } > >+sbPanel.prototype.is_in_view = >+function() >+{ >+ return (this.header.getAttribute("in-view") == "true"); >+} >+ > sbPanel.prototype.is_selected = > function (panel_id) > { > return 'true' == this.get_header().getAttribute('selected'); > } > >@@ -645,12 +714,24 @@ > sidebarObj.never_built = false; > > debug("sidebar = " + sidebarObj); > debug("sidebarObj.resource = " + sidebarObj.resource); > debug("sidebarObj.datasource_uri = " + sidebarObj.datasource_uri); > >+ // Obtain the pref for limiting the number of tabs in view >+ try >+ { >+ var prefs = Components.classes["@mozilla.org/preferences-service;1"]. >+ getService(Components.interfaces.nsIPrefBranch); >+ gNumTabsInViewPref = prefs.getIntPref("sidebar.num_tabs_in_view"); >+ } >+ catch (ex) >+ { >+ gNumTabsInViewPref = 6; // failover default >+ } >+ > // Show the header for the panels area. Use a splitter if there > // is stuff over the panels area. > var sidebar_panels_splitter = document.getElementById('sidebar-panels-splitter'); > if (sidebar_element.firstChild != sidebar_panels_splitter) { > debug("Showing the panels splitter"); > sidebar_panels_splitter.removeAttribute('hidden'); >@@ -1128,21 +1209,171 @@ > RDF.GetResource(NC + "exclude"), > exclude_target, > RDF.GetLiteral(new_exclude)); > } > } > >+ // if we ex/included a tab in view then add/remove another one >+ if (panel.is_in_view()) >+ { >+ if (did_exclude) >+ { >+ // we excluded one so let's try to bring a non-excluded one into view >+ var tabs = sidebarObj.panels.node.childNodes; >+ var newFirst = null; >+ for (var i = 2; i < tabs.length ; i += 2) >+ { >+ var currTab = sidebarObj.panels.get_panel_from_header_index(i); >+ var hasPotential = !currTab.is_excluded() && !currTab.is_in_view(); >+ >+ // set potential new first tab in case we can't find one after the >+ // tab that was just excluded >+ if (!newFirst && hasPotential) >+ newFirst = currTab; >+ >+ if (i > panel.index && hasPotential) >+ { >+ currTab.header.setAttribute("in-view", true); >+ added = true; >+ break; >+ } >+ } >+ if (!added && newFirst) >+ newFirst.header.setAttribute("in-view", true); >+ } >+ else >+ { >+ // we included a new tab so let's take the last one out of view >+ var tabs = sidebarObj.panels.node.childNodes; >+ for (i = 2; i < tabs.length; i += 2) >+ { >+ var currHeader = tabs[i]; >+ if (currHeader.hasAttribute("last-panel")) >+ currHeader.setAttribute("in-view", false); >+ } >+ } >+ } >+ > if (did_exclude && !sidebarObj.panels.visible_panels_exist()) > // surrender focus to main content area > window._content.focus(); > else > // force all the sidebars to update > refresh_all_sidebars(); > > // Write the modified panels out. > sidebarObj.datasource.QueryInterface(Components.interfaces.nsIRDFRemoteDataSource).Flush(); >+} >+ >+function SidebarNavigate(aDirection) >+{ >+ debug("SidebarNavigate " + aDirection); >+ >+ var tabs = sidebarObj.panels.node.childNodes; >+ // move forward a tab (down in the template) >+ if (aDirection > 0) >+ { >+ // ensure we have a tab below the last one >+ var foundLast = false; >+ var oldFirst = null; >+ for (var i = 2; i < tabs.length; i += 2) >+ { >+ var currHeader = tabs[i]; >+ var currTab = new sbPanel(currHeader.getAttribute("id"), currHeader, i); >+ >+ if (!currTab.is_excluded()) >+ { >+ if (foundLast) >+ { >+ debug("toggling old first and new last"); >+ debug("new last: " + currHeader.getAttribute("id")); >+ debug("old first: " + oldFirst.getAttribute("id")); >+ currHeader.setAttribute("in-view", true); >+ oldFirst.setAttribute("in-view", false); >+ >+ // if old first was selected select new first instead >+ if (oldFirst.getAttribute("id") == >+ sidebarObj.panels.node.getAttribute("last-selected-panel")) >+ { >+ sidebarObj.panels.node.setAttribute('last-selected-panel', >+ currTab.id); >+ } >+ >+ break; >+ } >+ >+ if (!foundLast && currHeader.hasAttribute("last-panel")) >+ { >+ debug("found last"); >+ foundLast = true; >+ } >+ >+ // set the old first in case we find a new last below >+ // the old last and need to toggle the new first's ``in-view'' >+ if (!oldFirst && currTab.is_in_view()) >+ oldFirst = currHeader; >+ } >+ } >+ } >+ >+ // move back a tab (up in the template) >+ else if (aDirection < 0) >+ { >+ var newFirst = null, newLast = null; >+ var foundFirst = false; >+ for (var i = 2; i < tabs.length; i += 2) >+ { >+ var currHeader = tabs[i]; >+ var currTab = new sbPanel(currHeader.getAttribute("id"), currHeader, i); >+ >+ if (!currTab.is_excluded()) >+ { >+ if (!foundFirst && currHeader.hasAttribute("top-panel")) >+ { >+ debug("found first"); >+ foundFirst = true; >+ } >+ if (!foundFirst) >+ { >+ debug("setting newFirst"); >+ newFirst = currHeader; >+ } >+ >+ if (currHeader.hasAttribute("last-panel")) >+ { >+ debug("found last"); >+ >+ // ensure we have a tab above the first one >+ if (newFirst) >+ { >+ debug("toggling new first and old last"); >+ debug("new first: " + newFirst.getAttribute("id")); >+ debug("old last: " + currHeader.getAttribute("id")); >+ >+ newFirst.setAttribute("in-view", true); >+ currHeader.setAttribute("in-view", false); // hide old last >+ >+ // if old last was selected, now select one above it >+ if (sidebarObj.panels.node.getAttribute("last-selected-panel") == >+ currTab.id) >+ { >+ sidebarObj.panels.node.setAttribute("last-selected-panel", >+ newLast.getAttribute("id")); >+ } >+ >+ break; >+ } >+ } >+ if (currTab.is_in_view()) >+ newLast = currHeader; >+ } >+ } >+ } >+ >+ if (aDirection) >+ sidebarObj.panels.update(false); > } > > ////////////////////////////////////////////////////////////// > // Sidebar Hacks and Work-arounds > ////////////////////////////////////////////////////////////// > >Index: modules/libpref/src/init/all.js >=================================================================== >RCS file: /cvsroot/mozilla/modules/libpref/src/init/all.js,v >retrieving revision 3.323 >diff -u -6 -r3.323 all.js >--- modules/libpref/src/init/all.js 14 Feb 2002 23:14:53 -0000 3.323 >+++ modules/libpref/src/init/all.js 18 Feb 2002 01:23:54 -0000 >@@ -571,12 +571,13 @@ > // Customizable toolbar stuff > pref("custtoolbar.personal_toolbar_folder", ""); > > pref("sidebar.customize.all_panels.url", "http://sidebar-rdf.netscape.com/%LOCALE%/sidebar-rdf/%SIDEBAR_VERSION%/all-panels.rdf"); > pref("sidebar.customize.directory.url", "http://dmoz.org/Netscape/Sidebar/"); > pref("sidebar.customize.more_panels.url", "http://dmoz.org/Netscape/Sidebar/"); >+pref("sidebar.num_tabs_in_view", 6); > > pref("prefs.converted-to-utf8",false); > // -------------------------------------------------- > // IBMBIDI > // -------------------------------------------------- > // >Index: themes/modern/communicator/sidebar/sidebar.css >=================================================================== >RCS file: /cvsroot/mozilla/themes/modern/communicator/sidebar/sidebar.css,v >retrieving revision 1.21 >diff -u -6 -r1.21 sidebar.css >--- themes/modern/communicator/sidebar/sidebar.css 7 Feb 2002 00:29:42 -0000 1.21 >+++ themes/modern/communicator/sidebar/sidebar.css 18 Feb 2002 01:23:55 -0000 >@@ -244,6 +244,13 @@ > list-style-image: url("chrome://communicator/skin/sidebar/sbtab-rit-top-sel.gif") !important; > } > > .box-texttab[selected] > .sidebar-tab-right-box > .sidebar-tab-right-btm { > background: #D2D8E3 !important; > } >+ >+/* ::::: sidebar navigation buttons ::::: */ >+ >+.sidebar-nav-button { >+ -moz-binding: url("chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton-image"); >+ padding: 5px 0px 5px 0px; >+} >Index: themes/classic/communicator/sidebar/sidebar.css >=================================================================== >RCS file: /cvsroot/mozilla/themes/classic/communicator/sidebar/sidebar.css,v >retrieving revision 1.4 >diff -u -6 -r1.4 sidebar.css >--- themes/classic/communicator/sidebar/sidebar.css 7 Feb 2002 00:29:43 -0000 1.4 >+++ themes/classic/communicator/sidebar/sidebar.css 18 Feb 2002 01:23:57 -0000 >@@ -150,8 +150,13 @@ > } > > .sbtab-twisty[selected="true"] { > list-style-image: url("chrome://communicator/skin/sidebar/sbtab-twisty-open.gif"); > } > >+/* ::::: sidebar navigation buttons ::::: */ > >+.sidebar-nav-button { >+ -moz-binding: url("chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton-image"); >+ padding: 5px 0px 5px 0px; >+} >

Comment on attachment 70014 [details] [diff] [review] Patch v1 >Index: xpfe/components/sidebar/resources/sidebarOverlay.xul >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/components/sidebar/resources/sidebarOverlay.xul,v >retrieving revision 1.81 >diff -u -6 -r1.81 sidebarOverlay.xul >--- xpfe/components/sidebar/resources/sidebarOverlay.xul 15 Feb 2002 12:33:30 -0000 1.81 >+++ xpfe/components/sidebar/resources/sidebarOverlay.xul 18 Feb 2002 01:23:52 -0000 >@@ -163,12 +163,20 @@ > <description>&sidebar.no-panels.state;</description> > <description>&sidebar.no-panels.add;</description> > <description>&sidebar.no-panels.hide;</description> > </vbox> > </vbox> > </vbox> >+ <hbox id="nav-buttons-box" hidden="true"> >+ <toolbarbutton flex="1" pack="center" class="sidebar-nav-button" >+ onclick="SidebarNavigate(-1);" >+ image="chrome://global/skin/arrow/arrow-up.gif"/> >+ <toolbarbutton flex="1" pack="center" class="sidebar-nav-button" >+ onclick="SidebarNavigate(1);" >+ image="chrome://global/skin/arrow/arrow-dn.gif"/> >+ </hbox> > <popupset id="contentAreaContextSet"/> > </vbox> > > <!-- Splitter on the right of sidebar --> > <splitter id="sidebar-splitter" collapse="before" persist="state hidden" > class="chromeclass-extrachrome sidebar-splitter" align="center" >Index: xpfe/components/sidebar/resources/sidebarOverlay.js >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/components/sidebar/resources/sidebarOverlay.js,v >retrieving revision 1.95 >diff -u -6 -r1.95 sidebarOverlay.js >--- xpfe/components/sidebar/resources/sidebarOverlay.js 7 Feb 2002 22:42:53 -0000 1.95 >+++ xpfe/components/sidebar/resources/sidebarOverlay.js 18 Feb 2002 01:23:53 -0000 >@@ -51,12 +51,16 @@ > } > > > // Uncomment for debug output > const SB_DEBUG = false; > >+// pref for limiting number of tabs in view >+// initialized in sidebar_overlay_init() >+var gNumTabsInViewPref; >+ > // The rdf service > const RDF_URI = '@mozilla.org/rdf/rdf-service;1'; > var RDF = Components.classes[RDF_URI].getService(); > RDF = RDF.QueryInterface(Components.interfaces.nsIRDFService); > > const NC = "http://home.netscape.com/NC-rdf#"; >@@ -81,12 +85,13 @@ > > function sbPanelList(container_id) > { > debug("sbPanelList("+container_id+")"); > this.node = document.getElementById(container_id); > this.childNodes = this.node.childNodes; >+ this.initialized = false; // set after first display of tabs > } > > sbPanelList.prototype.get_panel_from_id = > function (id) > { > debug("get_panel_from_id(" + id + ")"); >@@ -124,26 +129,26 @@ > sbPanelList.prototype.find_first = > function (panels) > { > debug("pick_default_panel: length=" + this.node.childNodes.length); > for (var ii = 2; ii < this.node.childNodes.length; ii += 2) { > var panel = this.get_panel_from_header_index(ii); >- if (!panel.is_excluded()) { >+ if (!panel.is_excluded() && panel.is_in_view()) { > return panel; > } > } > return null; > } > > sbPanelList.prototype.find_last = > function (panels) > { > debug("pick_default_panel: length=" + this.node.childNodes.length); > for (var ii=(this.node.childNodes.length - 1); ii >= 2; ii -= 2) { > var panel = this.get_panel_from_header_index(ii); >- if (!panel.is_excluded()) { >+ if (!panel.is_excluded() && panel.is_in_view()) { > return panel; > } > } > return null; > } > >@@ -157,12 +162,26 @@ > if (!panels.item(i).hidden) > return true; > } > return false; > } > >+sbPanelList.prototype.num_included_panels = >+function () >+{ >+ var count = 0; >+ var panels = this.node.childNodes; >+ for (var i = 2; i < panels.length; i += 2) >+ { >+ var curr = this.get_panel_from_header_index(i); >+ if (!curr.is_excluded()) >+ count++; >+ } >+ return count; >+} >+ > sbPanelList.prototype.select = > function (panel, force_reload) > { > if (!force_reload && panel.is_selected()) { > return; > } >@@ -194,23 +213,23 @@ > > // First, check the XUL for the "defaultpanel" attribute of "sidebar-box". > var sidebar_container = document.getElementById('sidebar-box'); > var content_default_id = sidebar_container.getAttribute('defaultpanel'); > if (content_default_id != '') { > var content = sidebarObj.panels.get_panel_from_id(content_default_id); >- if (content && !content.is_excluded()) { >+ if (content && !content.is_excluded() && content.is_in_view()) { > default_panel = content; > } > } > > // Second, try to use the panel persisted in 'last-selected-panel'. > if (!default_panel) { > var last_selected_id = this.node.getAttribute('last-selected-panel'); > if (last_selected_id != '') { > var last = sidebarObj.panels.get_panel_from_id(last_selected_id); >- if (last && !last.is_excluded()) { >+ if (last && !last.is_excluded() && last.is_in_view()) { > default_panel = last; > } > } > } > > // Finally, just use the last one in the list. >@@ -264,41 +283,83 @@ > if (sidebar_is_collapsed()) { > sidebarObj.collapsed = true; > } else { > sidebarObj.collapsed = false; > } > >+ if (sidebarObj.panels.num_included_panels() > gNumTabsInViewPref) >+ document.getElementById("nav-buttons-box").hidden = false; >+ else >+ document.getElementById("nav-buttons-box").hidden = true; >+ > var have_set_top = 0; > var have_set_after_selected = 0; > var is_after_selected = 0; > var last_header = 0; >+ var num_in_view = 0; >+ debug("this.initialized: " + this.initialized); > for (var ii=2; ii < this.node.childNodes.length; ii += 2) { > var header = this.node.childNodes.item(ii); > var content = this.node.childNodes.item(ii+1); > var id = header.getAttribute('id'); > var panel = new sbPanel(id, header, ii); >- if (panel.is_excluded()) { >- debug("item("+ii/2+") excluded"); >+ var excluded = panel.is_excluded(); >+ var in_view = false; >+ if (!this.initialized) >+ { >+ if (num_in_view < gNumTabsInViewPref) >+ in_view = true; >+ } >+ else >+ { >+ if (header.getAttribute("in-view") == "true") >+ in_view = true; >+ } >+ if (excluded || !in_view) >+ { >+ debug("item("+ii/2+") excluded: " + excluded + >+ " in view: " + in_view); > header.setAttribute('hidden','true'); > content.setAttribute('hidden','true'); >+ if (!in_view) >+ { >+ header.setAttribute("in-view", false); >+ header.removeAttribute("top-panel"); >+ header.removeAttribute("last-panel"); >+ } > } else { >- last_header = header; >+ // only set if in view >+ if (!this.initialized || (num_in_view < gNumTabsInViewPref)) >+ last_header = header; > header.removeAttribute('last-panel'); >- if (!have_set_top) { >+ // only set if in view >+ if (!have_set_top && >+ (!this.initialized || (header.getAttribute("in-view") == "true"))) >+ { > header.setAttribute('top-panel','true'); >- have_set_top = 1 >+ have_set_top = 1; > } else { > header.removeAttribute('top-panel'); > } > if (!have_set_after_selected && is_after_selected) { > header.setAttribute('first-panel-after-selected','true'); > have_set_after_selected = 1 > } else { > header.removeAttribute('first-panel-after-selected'); > } > header.removeAttribute('hidden'); >+ header.setAttribute("in-view", true); >+ num_in_view++; >+ >+ // selected tab is not in view so just select the last one >+ if (!is_after_selected && num_in_view == gNumTabsInViewPref && >+ selected_id != id) >+ { >+ selected_id = id; >+ this.node.setAttribute('last-selected-panel', id); >+ } > > // Pick sandboxed, or unsandboxed iframe > var iframe = panel.get_iframe(); > var load_state; > > if (selected_id == id) { >@@ -372,12 +433,14 @@ > // but some old profiles may have this persisted as hidden (50973). > this.node.removeAttribute('hidden'); > } else { > no_panels_iframe.removeAttribute('hidden'); > this.node.setAttribute('hidden','true'); > } >+ >+ this.initialized = true; > } > > > ////////////////////////////////////////////////////////////////////// > // sbPanel Class > // >@@ -452,12 +515,18 @@ > sbPanel.prototype.is_excluded = > function () > { > return sb_panel_is_excluded(this.get_header()); > } > >+sbPanel.prototype.is_in_view = >+function() >+{ >+ return (this.header.getAttribute("in-view") == "true"); >+} >+ > sbPanel.prototype.is_selected = > function (panel_id) > { > return 'true' == this.get_header().getAttribute('selected'); > } > >@@ -645,12 +714,24 @@ > sidebarObj.never_built = false; > > debug("sidebar = " + sidebarObj); > debug("sidebarObj.resource = " + sidebarObj.resource); > debug("sidebarObj.datasource_uri = " + sidebarObj.datasource_uri); > >+ // Obtain the pref for limiting the number of tabs in view >+ try >+ { >+ var prefs = Components.classes["@mozilla.org/preferences-service;1"]. >+ getService(Components.interfaces.nsIPrefBranch); >+ gNumTabsInViewPref = prefs.getIntPref("sidebar.num_tabs_in_view"); >+ } >+ catch (ex) >+ { >+ gNumTabsInViewPref = 6; // failover default >+ } >+ > // Show the header for the panels area. Use a splitter if there > // is stuff over the panels area. > var sidebar_panels_splitter = document.getElementById('sidebar-panels-splitter'); > if (sidebar_element.firstChild != sidebar_panels_splitter) { > debug("Showing the panels splitter"); > sidebar_panels_splitter.removeAttribute('hidden'); >@@ -1128,21 +1209,171 @@ > RDF.GetResource(NC + "exclude"), > exclude_target, > RDF.GetLiteral(new_exclude)); > } > } > >+ // if we ex/included a tab in view then add/remove another one >+ if (panel.is_in_view()) >+ { >+ if (did_exclude) >+ { >+ // we excluded one so let's try to bring a non-excluded one into view >+ var tabs = sidebarObj.panels.node.childNodes; >+ var newFirst = null; >+ for (var i = 2; i < tabs.length ; i += 2) >+ { >+ var currTab = sidebarObj.panels.get_panel_from_header_index(i); >+ var hasPotential = !currTab.is_excluded() && !currTab.is_in_view(); >+ >+ // set potential new first tab in case we can't find one after the >+ // tab that was just excluded >+ if (!newFirst && hasPotential) >+ newFirst = currTab; >+ >+ if (i > panel.index && hasPotential) >+ { >+ currTab.header.setAttribute("in-view", true); >+ added = true; >+ break; >+ } >+ } >+ if (!added && newFirst) >+ newFirst.header.setAttribute("in-view", true); >+ } >+ else >+ { >+ // we included a new tab so let's take the last one out of view >+ var tabs = sidebarObj.panels.node.childNodes; >+ for (i = 2; i < tabs.length; i += 2) >+ { >+ var currHeader = tabs[i]; >+ if (currHeader.hasAttribute("last-panel")) >+ currHeader.setAttribute("in-view", false); >+ } >+ } >+ } >+ > if (did_exclude && !sidebarObj.panels.visible_panels_exist()) > // surrender focus to main content area > window._content.focus(); > else > // force all the sidebars to update > refresh_all_sidebars(); > > // Write the modified panels out. > sidebarObj.datasource.QueryInterface(Components.interfaces.nsIRDFRemoteDataSource).Flush(); >+} >+ >+function SidebarNavigate(aDirection) >+{ >+ debug("SidebarNavigate " + aDirection); >+ >+ var tabs = sidebarObj.panels.node.childNodes; >+ // move forward a tab (down in the template) >+ if (aDirection > 0) >+ { >+ // ensure we have a tab below the last one >+ var foundLast = false; >+ var oldFirst = null; >+ for (var i = 2; i < tabs.length; i += 2) >+ { >+ var currHeader = tabs[i]; >+ var currTab = new sbPanel(currHeader.getAttribute("id"), currHeader, i); >+ >+ if (!currTab.is_excluded()) >+ { >+ if (foundLast) >+ { >+ debug("toggling old first and new last"); >+ debug("new last: " + currHeader.getAttribute("id")); >+ debug("old first: " + oldFirst.getAttribute("id")); >+ currHeader.setAttribute("in-view", true); >+ oldFirst.setAttribute("in-view", false); >+ >+ // if old first was selected select new first instead >+ if (oldFirst.getAttribute("id") == >+ sidebarObj.panels.node.getAttribute("last-selected-panel")) >+ { >+ sidebarObj.panels.node.setAttribute('last-selected-panel', >+ currTab.id); >+ } >+ >+ break; >+ } >+ >+ if (!foundLast && currHeader.hasAttribute("last-panel")) >+ { >+ debug("found last"); >+ foundLast = true; >+ } >+ >+ // set the old first in case we find a new last below >+ // the old last and need to toggle the new first's ``in-view'' >+ if (!oldFirst && currTab.is_in_view()) >+ oldFirst = currHeader; >+ } >+ } >+ } >+ >+ // move back a tab (up in the template) >+ else if (aDirection < 0) >+ { >+ var newFirst = null, newLast = null; >+ var foundFirst = false; >+ for (var i = 2; i < tabs.length; i += 2) >+ { >+ var currHeader = tabs[i]; >+ var currTab = new sbPanel(currHeader.getAttribute("id"), currHeader, i); >+ >+ if (!currTab.is_excluded()) >+ { >+ if (!foundFirst && currHeader.hasAttribute("top-panel")) >+ { >+ debug("found first"); >+ foundFirst = true; >+ } >+ if (!foundFirst) >+ { >+ debug("setting newFirst"); >+ newFirst = currHeader; >+ } >+ >+ if (currHeader.hasAttribute("last-panel")) >+ { >+ debug("found last"); >+ >+ // ensure we have a tab above the first one >+ if (newFirst) >+ { >+ debug("toggling new first and old last"); >+ debug("new first: " + newFirst.getAttribute("id")); >+ debug("old last: " + currHeader.getAttribute("id")); >+ >+ newFirst.setAttribute("in-view", true); >+ currHeader.setAttribute("in-view", false); // hide old last >+ >+ // if old last was selected, now select one above it >+ if (sidebarObj.panels.node.getAttribute("last-selected-panel") == >+ currTab.id) >+ { >+ sidebarObj.panels.node.setAttribute("last-selected-panel", >+ newLast.getAttribute("id")); >+ } >+ >+ break; >+ } >+ } >+ if (currTab.is_in_view()) >+ newLast = currHeader; >+ } >+ } >+ } >+ >+ if (aDirection) >+ sidebarObj.panels.update(false); > } > > ////////////////////////////////////////////////////////////// > // Sidebar Hacks and Work-arounds > ////////////////////////////////////////////////////////////// > >Index: modules/libpref/src/init/all.js >=================================================================== >RCS file: /cvsroot/mozilla/modules/libpref/src/init/all.js,v >retrieving revision 3.323 >diff -u -6 -r3.323 all.js >--- modules/libpref/src/init/all.js 14 Feb 2002 23:14:53 -0000 3.323 >+++ modules/libpref/src/init/all.js 18 Feb 2002 01:23:54 -0000 >@@ -571,12 +571,13 @@ > // Customizable toolbar stuff > pref("custtoolbar.personal_toolbar_folder", ""); > > pref("sidebar.customize.all_panels.url", "http://sidebar-rdf.netscape.com/%LOCALE%/sidebar-rdf/%SIDEBAR_VERSION%/all-panels.rdf"); > pref("sidebar.customize.directory.url", "http://dmoz.org/Netscape/Sidebar/"); > pref("sidebar.customize.more_panels.url", "http://dmoz.org/Netscape/Sidebar/"); >+pref("sidebar.num_tabs_in_view", 6); > > pref("prefs.converted-to-utf8",false); > // -------------------------------------------------- > // IBMBIDI > // -------------------------------------------------- > // >Index: themes/modern/communicator/sidebar/sidebar.css >=================================================================== >RCS file: /cvsroot/mozilla/themes/modern/communicator/sidebar/sidebar.css,v >retrieving revision 1.21 >diff -u -6 -r1.21 sidebar.css >--- themes/modern/communicator/sidebar/sidebar.css 7 Feb 2002 00:29:42 -0000 1.21 >+++ themes/modern/communicator/sidebar/sidebar.css 18 Feb 2002 01:23:55 -0000 >@@ -244,6 +244,13 @@ > list-style-image: url("chrome://communicator/skin/sidebar/sbtab-rit-top-sel.gif") !important; > } > > .box-texttab[selected] > .sidebar-tab-right-box > .sidebar-tab-right-btm { > background: #D2D8E3 !important; > } >+ >+/* ::::: sidebar navigation buttons ::::: */ >+ >+.sidebar-nav-button { >+ -moz-binding: url("chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton-image"); >+ padding: 5px 0px 5px 0px; >+} >Index: themes/classic/communicator/sidebar/sidebar.css >=================================================================== >RCS file: /cvsroot/mozilla/themes/classic/communicator/sidebar/sidebar.css,v >retrieving revision 1.4 >diff -u -6 -r1.4 sidebar.css >--- themes/classic/communicator/sidebar/sidebar.css 7 Feb 2002 00:29:43 -0000 1.4 >+++ themes/classic/communicator/sidebar/sidebar.css 18 Feb 2002 01:23:57 -0000 >@@ -150,8 +150,13 @@ > } > > .sbtab-twisty[selected="true"] { > list-style-image: url("chrome://communicator/skin/sidebar/sbtab-twisty-open.gif"); > } > >+/* ::::: sidebar navigation buttons ::::: */ > >+.sidebar-nav-button { >+ -moz-binding: url("chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton-image"); >+ padding: 5px 0px 5px 0px; >+} >

Sorry guys, don't use today's build to edit attachments :-( I hit "Edit attachment as Comment" and then first deletion I made (by selecting and using the Enter key to open up space) submitted the form.

Really sorry for the earlier crud. I'll do my editing straight into the main bugzilla comment box for now. >+ <hbox id="nav-buttons-box" hidden="true"> >+ <toolbarbutton flex="1" pack="center" class="sidebar-nav-button" >+ onclick="SidebarNavigate(-1);" >+ image="chrome://global/skin/arrow/arrow-up.gif"/> >+ <toolbarbutton flex="1" pack="center" class="sidebar-nav-button" >+ onclick="SidebarNavigate(1);" >+ image="chrome://global/skin/arrow/arrow-dn.gif"/> >+ </hbox> Raw image URLS are forbidden in XUL. See http://www.mozilla.org/xpfe/skins.html and other docs at http://www.mozilla.org/xpfe/ >+pref("sidebar.num_tabs_in_view", 6); .... >+ gNumTabsInViewPref = 6; // failover default You only allow 6?! Without a way for the user to adjust this, and with scrolling moving a tab at a time instead of a page-full? Wow, I think I will kill myself. 8 was pretty uncomfortable at 800x600 but still worked. at larger resolutions 6 is way too miserly. I currently have 14 tabs and plenty of space for them. I'd hate being restricted to 6. The URLs in XUL need to be fixed. The rest is kind of a matter of opinion, is there a spec somewhere that shows people have thought out these defaults and are prepared for the firestorm of hatred that is about to land on your head? I'd like at least to see bugs filed on exposing the pref in the UI (the customize sidebar dialog?) and another on adding "fast forward" buttons.

Filed bug 126537 regarding exposing the number of tabs. We can discuss the default in that bug.

Filed bug 126559 about the fast forward/rewind buttons.

dveditz, please sr.

Comment on attachment 70472 [details] [diff] [review] Patch v2: moved image URLs into CSS. sr=dveditz (though I still think 6 is too few).

Checked in. Bumped up default num tabs in view to 8.

verified...