Commit 0eef9bbe authored by John Lee's avatar John Lee Committed by Commit Bot

WebUI Tab Strip: Update scroll interactions

This change prioritizes scrolling to tabs created by the New Tab
button and prevents scrolling when activating tabs directly to prevent
unnecessary scrolling when the user has already found and activated
a tab.

Bug: 1023492
Change-Id: I5e6a7c6744582cfc51eb8a15c5ec5dd214e96b52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1947627
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721341}
parent 46fcaeb8
......@@ -170,10 +170,6 @@ class TabListElement extends CustomElement {
this.tabsApi_.getTabs().then(tabs => {
tabs.forEach(tab => this.onTabCreated_(tab));
this.animationPromises.then(() => {
this.scrollToActiveTab_();
});
addWebUIListener('tab-created', tab => this.onTabCreated_(tab));
addWebUIListener(
'tab-moved', (tabId, newIndex) => this.onTabMoved_(tabId, newIndex));
......@@ -272,7 +268,9 @@ class TabListElement extends CustomElement {
/** @private */
onDocumentVisibilityChange_() {
this.scrollToActiveTab_();
if (!this.tabStripEmbedderProxy_.isVisible()) {
this.scrollToActiveTab_();
}
Array.from(this.tabsContainerElement_.children)
.forEach((tabElement) => this.updateThumbnailTrackStatus_(tabElement));
}
......@@ -372,7 +370,6 @@ class TabListElement extends CustomElement {
if (newlyActiveTab) {
newlyActiveTab.tab = /** @type {!TabData} */ (
Object.assign({}, newlyActiveTab.tab, {active: true}));
this.scrollToActiveTab_();
}
}
......@@ -398,6 +395,9 @@ class TabListElement extends CustomElement {
const tabElement = this.createTabElement_(tab);
this.insertTabOrMoveTo_(tabElement, tab.index);
this.addAnimationPromise_(tabElement.slideIn());
if (tab.active) {
this.scrollToTab_(tabElement);
}
}
/**
......@@ -461,7 +461,6 @@ class TabListElement extends CustomElement {
if (tab.active) {
this.scrollToTab_(tabElement);
}
this.updateThumbnailTrackStatus_(tabElement);
}
}
......
......@@ -292,44 +292,6 @@ suite('TabList', () => {
assertEquals(tabElementsBeforeMove[2], tabElementsAfterMove[1]);
});
test('activating a tab off-screen scrolls to it', async () => {
testTabStripEmbedderProxy.setVisible(true);
const scrollPadding = 32;
// Mock the width of each tab element
const tabElements = getUnpinnedTabs();
tabElements.forEach((tabElement) => {
tabElement.style.width = '200px';
});
// Mock the scrolling parent such that it cannot fit only 1 tab at a time
const fakeScroller = {
offsetWidth: 300,
scrollLeft: 0,
};
tabList.scrollingParent_ = fakeScroller;
// The 2nd tab should be off-screen to the right, so activating it should
// scroll so that the element's right edge is aligned with the screen's
// right edge
webUIListenerCallback('tab-active-changed', tabs[1].id);
let activeTab = getUnpinnedTabs()[1];
await tabList.animationPromises;
assertEquals(
fakeScroller.scrollLeft,
activeTab.offsetLeft + activeTab.offsetWidth -
fakeScroller.offsetWidth + scrollPadding);
// The 1st tab should be now off-screen to the left, so activating it should
// scroll so that the element's left edge is aligned with the screen's
// left edge
webUIListenerCallback('tab-active-changed', tabs[0].id);
activeTab = getUnpinnedTabs()[0];
await tabList.animationPromises;
assertEquals(fakeScroller.scrollLeft, activeTab.offsetLeft - scrollPadding);
});
test('dragstart sets a drag image offset by the event coordinates', () => {
// Drag and drop only works for pinned tabs
tabs.forEach(pinTabAt);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment