From fdd5780dabb33e933fd3de2c2f83727f1f3a734b Mon Sep 17 00:00:00 2001 From: Milen Karmidzhanov Date: Tue, 2 Sep 2025 15:49:47 +0300 Subject: [PATCH 1/4] fix(ui5-tokenizer): scroll to end when expanded --- packages/main/src/MultiComboBox.ts | 5 ---- packages/main/src/MultiInput.ts | 9 +------ packages/main/src/Tokenizer.ts | 38 ++++++++++++++++++++++++++---- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/packages/main/src/MultiComboBox.ts b/packages/main/src/MultiComboBox.ts index cf45801924c9..881dbfbffcc7 100644 --- a/packages/main/src/MultiComboBox.ts +++ b/packages/main/src/MultiComboBox.ts @@ -1752,10 +1752,6 @@ class MultiComboBox extends UI5Element implements IFormInputElement { this._tokenizer.expanded = true; } - if (this._tokenizer.expanded && this.hasAttribute("focused")) { - this._tokenizer.scrollToEnd(); - } - if (!arraysAreEqual(this._valueStateLinks, this.linksInAriaValueStateHiddenText)) { this._removeLinksEventListeners(); this._addLinksEventListeners(); @@ -1854,7 +1850,6 @@ class MultiComboBox extends UI5Element implements IFormInputElement { if (!isPhone()) { this.focused = true; this._tokenizer.expanded = true; - this._tokenizer.scrollToEnd(); } else { this._innerInput.blur(); } diff --git a/packages/main/src/MultiInput.ts b/packages/main/src/MultiInput.ts index f65b278108b4..5782bd44c759 100644 --- a/packages/main/src/MultiInput.ts +++ b/packages/main/src/MultiInput.ts @@ -197,7 +197,7 @@ class MultiInput extends Input implements IFormInputElement { _tokenizerFocusOut(e: FocusEvent) { if (!this.contains(e.relatedTarget as HTMLElement) && !this.shadowRoot!.contains(e.relatedTarget as HTMLElement)) { this.tokenizer._tokens.forEach(token => { token.selected = false; }); - this.tokenizer.scrollToStart(); + // this.tokenizer.scrollToStart(); } } @@ -210,7 +210,6 @@ class MultiInput extends Input implements IFormInputElement { innerFocusIn() { this.tokenizer.expanded = true; this.focused = true; - this.tokenizer.scrollToEnd(); this.tokens.forEach(token => { token.selected = false; @@ -340,12 +339,6 @@ class MultiInput extends Input implements IFormInputElement { super.onAfterRendering(); this.tokenizer.preventInitialFocus = true; - - if (this.tokenizer.expanded) { - this.tokenizer.scrollToEnd(); - } else { - this.tokenizer.scrollToStart(); - } } get iconsCount() { diff --git a/packages/main/src/Tokenizer.ts b/packages/main/src/Tokenizer.ts index 0a8f631d3ed7..3336f69e84a5 100644 --- a/packages/main/src/Tokenizer.ts +++ b/packages/main/src/Tokenizer.ts @@ -352,6 +352,8 @@ class Tokenizer extends UI5Element { _previousToken: Token | null = null; _focusedElementBeforeOpen?: HTMLElement | null; _deletedDialogItems!: Token[]; + _lastTokenCount?: number; + _lastExpanded?: boolean; _handleResize() { this._nMoreCount = this.overflownTokens.length; @@ -405,7 +407,6 @@ class Tokenizer extends UI5Element { if (!this.preventPopoverOpen) { this.open = true; - this.scrollToEnd(); } this._tokens.forEach(token => { @@ -476,9 +477,30 @@ class Tokenizer extends UI5Element { this._expandedScrollWidth = this.contentDom.scrollWidth; } + this._scrollToEndIfNeeded(); this._tokenDeleting = false; } + _scrollToEndIfNeeded() { + const tokenCount = this.tokens.length; + if (this._lastTokenCount === undefined) { + this._lastTokenCount = tokenCount; + } + if (this._lastExpanded === undefined) { + this._lastExpanded = this.expanded; + } + + const tokenAdded = tokenCount > this._lastTokenCount; + const expandedNow = this.expanded && !this._lastExpanded; + + if (tokenAdded || expandedNow) { + renderFinished().then(() => this.scrollToEnd()); + } + + this._lastTokenCount = tokenCount; + this._lastExpanded = this.expanded; + } + _delete(e: CustomEvent) { const target = e.target as Token; @@ -1006,7 +1028,8 @@ class Tokenizer extends UI5Element { /** * Scrolls token to the visible area of the container. - * Adds 4 pixels to the scroll position to ensure padding and border visibility on both ends + * Adds 5 pixels to the scroll position to ensure padding and border visibility on both ends + * For the last token, if its width is more than the needed space, scroll to the end without offset * @protected */ _scrollToToken(token: IToken) { @@ -1016,11 +1039,18 @@ class Tokenizer extends UI5Element { const tokenRect = token.getBoundingClientRect(); const tokenContainerRect = this.contentDom.getBoundingClientRect(); + const oneSideBborderAndPaddingOffset = 5; + + const isLastToken = this._tokens.indexOf(token as Token) === this._tokens.length - 1; + if (isLastToken) { + this.scrollToEnd(); + return; + } if (tokenRect.left < tokenContainerRect.left) { - this._scrollEnablement?.scrollTo(this.contentDom.scrollLeft - (tokenContainerRect.left - tokenRect.left + 5), 0); + this._scrollEnablement?.scrollTo(this.contentDom.scrollLeft - (tokenContainerRect.left - tokenRect.left + oneSideBborderAndPaddingOffset), 0); } else if (tokenRect.right > tokenContainerRect.right) { - this._scrollEnablement?.scrollTo(this.contentDom.scrollLeft + (tokenRect.right - tokenContainerRect.right + 5), 0); + this._scrollEnablement?.scrollTo(this.contentDom.scrollLeft + (tokenRect.right - tokenContainerRect.right + oneSideBborderAndPaddingOffset), 0); } } From 8d2937367eff82dc28e1568cd3cb6be5e09ec3f7 Mon Sep 17 00:00:00 2001 From: Milen Karmidzhanov Date: Tue, 2 Sep 2025 16:35:50 +0300 Subject: [PATCH 2/4] fix(ui5-tokenizer): scroll issues --- packages/main/src/MultiInput.ts | 1 - packages/main/src/Tokenizer.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/main/src/MultiInput.ts b/packages/main/src/MultiInput.ts index 5782bd44c759..27798db3fa44 100644 --- a/packages/main/src/MultiInput.ts +++ b/packages/main/src/MultiInput.ts @@ -197,7 +197,6 @@ class MultiInput extends Input implements IFormInputElement { _tokenizerFocusOut(e: FocusEvent) { if (!this.contains(e.relatedTarget as HTMLElement) && !this.shadowRoot!.contains(e.relatedTarget as HTMLElement)) { this.tokenizer._tokens.forEach(token => { token.selected = false; }); - // this.tokenizer.scrollToStart(); } } diff --git a/packages/main/src/Tokenizer.ts b/packages/main/src/Tokenizer.ts index a404afea7b09..a29040b4ab55 100644 --- a/packages/main/src/Tokenizer.ts +++ b/packages/main/src/Tokenizer.ts @@ -492,7 +492,7 @@ class Tokenizer extends UI5Element { const expandedNow = this.expanded && !this._lastExpanded; if (tokenAdded || expandedNow) { - renderFinished().then(() => this.scrollToEnd()); + this.scrollToEnd(); } this._lastTokenCount = tokenCount; From 6870251cd57fe43695310bc787096c2d12501234 Mon Sep 17 00:00:00 2001 From: Milen Karmidzhanov Date: Fri, 5 Sep 2025 13:37:13 +0300 Subject: [PATCH 3/4] fix(ui5-tokenizer): add tests --- packages/main/cypress/specs/Tokenizer.cy.tsx | 239 +++++++++++++++++++ packages/main/src/Tokenizer.ts | 41 ++-- 2 files changed, 258 insertions(+), 22 deletions(-) mode change 100644 => 100755 packages/main/cypress/specs/Tokenizer.cy.tsx diff --git a/packages/main/cypress/specs/Tokenizer.cy.tsx b/packages/main/cypress/specs/Tokenizer.cy.tsx old mode 100644 new mode 100755 index 3a2ab92cae00..c30ae1259df1 --- a/packages/main/cypress/specs/Tokenizer.cy.tsx +++ b/packages/main/cypress/specs/Tokenizer.cy.tsx @@ -803,6 +803,245 @@ describe("Accessibility", () => { }); }); +describe("Scrolling Behavior", () => { + it("should scroll to end when tokenizer is expanded without focused token", () => { + cy.mount( +
+ + + + + + + +
+ ); + + // Verify there's actually overflow to test + cy.get("[ui5-tokenizer]") + .shadow() + .find(".ui5-tokenizer--content") + .then($content => { + const element = $content[0]; + // Only proceed if there's actual overflow + if (element.scrollWidth > element.clientWidth) { + expect(element.scrollLeft).to.equal(0); + } + }); + + // Click on tokenizer container to expand it + cy.get("[ui5-tokenizer]") + .realClick(); + + // Verify tokenizer is expanded + cy.get("[ui5-tokenizer]") + .should("have.attr", "expanded"); + + // Should scroll to end when expanded without focused token (only if there's overflow) + cy.get("[ui5-tokenizer]") + .shadow() + .find(".ui5-tokenizer--content") + .then($content => { + const element = $content[0]; + const maxScroll = element.scrollWidth - element.clientWidth; + if (maxScroll > 0) { + // Should be scrolled to the end (or close to it) + expect(element.scrollLeft).to.be.greaterThan(maxScroll * 0.5); + } + }); + }); + + it("should scroll to specific token when token is clicked", () => { + cy.mount( +
+ + + + + + + +
+ ); + + // Click on the third token (middle token) + cy.get("[ui5-token]") + .eq(2) + .as("thirdToken") + .realClick(); + + // Verify token is selected and focused (separate assertions) + cy.get("@thirdToken") + .should("have.attr", "selected"); + + cy.get("@thirdToken") + .should("have.attr", "focused"); + + // Verify tokenizer is expanded + cy.get("[ui5-tokenizer]") + .should("have.attr", "expanded"); + + // Should scroll to show the selected token + cy.get("[ui5-tokenizer]") + .shadow() + .find(".ui5-tokenizer--content") + .then($content => { + const element = $content[0]; + const maxScroll = element.scrollWidth - element.clientWidth; + if (maxScroll > 0) { + // Should be scrolled some amount to show the token + expect(element.scrollLeft).to.be.greaterThan(0); + } + }); + }); + + it("should scroll when navigating with Home and End keys", () => { + cy.mount( +
+ + + + + + + +
+ ); + + // Click on first token + cy.get("[ui5-token]") + .eq(0) + .realClick(); + + // Navigate to the last token using End key + cy.realPress("End"); + + cy.get("[ui5-token]") + .eq(4) + .should("have.attr", "focused"); + + // Should scroll to end for last token + cy.get("[ui5-tokenizer]") + .shadow() + .find(".ui5-tokenizer--content") + .then($content => { + const element = $content[0]; + const maxScroll = element.scrollWidth - element.clientWidth; + if (maxScroll > 0) { + expect(element.scrollLeft).to.be.closeTo(maxScroll, 20); + } + }); + + // Navigate back to first token using Home key + cy.realPress("Home"); + + cy.get("[ui5-token]") + .eq(0) + .should("have.attr", "focused"); + + // Should scroll back to start for first token + cy.get("[ui5-tokenizer]") + .shadow() + .find(".ui5-tokenizer--content") + .then($content => { + const element = $content[0]; + expect(element.scrollLeft).to.be.closeTo(0, 20); + }); + }); + + it("should maintain scroll position when token selection is toggled", () => { + cy.mount( +
+ + + + + + + +
+ ); + + // Click on middle token to select it + cy.get("[ui5-token]") + .eq(2) + .as("middleToken") + .realClick(); + + // Get scroll position after clicking middle token + let scrollAfterClick; + cy.get("[ui5-tokenizer]") + .shadow() + .find(".ui5-tokenizer--content") + .then($content => { + scrollAfterClick = $content[0].scrollLeft; + }); + + // Toggle selection with space (should deselect but maintain focus) + cy.realPress("Space"); + + cy.get("@middleToken") + .should("not.have.attr", "selected"); + + cy.get("@middleToken") + .should("have.attr", "focused"); + + // Scroll position should remain the same since token is still focused + cy.get("[ui5-tokenizer]") + .shadow() + .find(".ui5-tokenizer--content") + .then($content => { + expect($content[0].scrollLeft).to.equal(scrollAfterClick); + }); + }); + + it("should scroll to end when tokenizer regains focus without focused token", () => { + cy.mount( +
+ + + + + + + + +
+ ); + + // Click on a token to expand tokenizer + cy.get("[ui5-token]") + .eq(1) + .realClick(); + + // Tab to external button (lose focus) + cy.realPress("Tab"); + + // Verify tokenizer is collapsed + cy.get("[ui5-tokenizer]") + .should("not.have.attr", "expanded"); + + // Tab back to tokenizer (regain focus) + cy.realPress(["Shift", "Tab"]); + + // Verify tokenizer is expanded again + cy.get("[ui5-tokenizer]") + .should("have.attr", "expanded"); + + // Should scroll to end when regaining focus (as per _scrollToEndIfNeeded logic) + cy.get("[ui5-tokenizer]") + .shadow() + .find(".ui5-tokenizer--content") + .then($content => { + const element = $content[0]; + const maxScroll = element.scrollWidth - element.clientWidth; + if (maxScroll > 0) { + expect(element.scrollLeft).to.be.greaterThan(maxScroll * 0.5); + } + }); + }); +}); + describe("Keyboard Handling", () => { beforeEach(() => { cy.mount( diff --git a/packages/main/src/Tokenizer.ts b/packages/main/src/Tokenizer.ts index a29040b4ab55..0d9df43d7a09 100644 --- a/packages/main/src/Tokenizer.ts +++ b/packages/main/src/Tokenizer.ts @@ -350,8 +350,7 @@ class Tokenizer extends UI5Element { _previousToken: Token | null = null; _focusedElementBeforeOpen?: HTMLElement | null; _deletedDialogItems!: Token[]; - _lastTokenCount?: number; - _lastExpanded?: boolean; + _focusedToken?: Token; _handleResize() { this._nMoreCount = this.overflownTokens.length; @@ -409,6 +408,7 @@ class Tokenizer extends UI5Element { this._tokens.forEach(token => { token.forcedTabIndex = "-1"; + this._focusedToken = token; }); this._skipTabIndex = true; @@ -419,14 +419,13 @@ class Tokenizer extends UI5Element { _onmousedown(e: MouseEvent) { if ((e.target as HTMLElement).hasAttribute("ui5-token")) { const target = e.target as Token; - this.expanded = true; if (this.open) { this._preventCollapse = true; } if (!target.toBeDeleted) { - this._itemNav.setCurrentItem(target); + this._addTokenToNavigation(target); this._scrollToToken(target); } } @@ -479,24 +478,20 @@ class Tokenizer extends UI5Element { this._tokenDeleting = false; } + /** + * Scrolls the container to the end to ensure very long tokens are visible at their end. + * Otherwise, tokens may appear visually cut off. + * @protected + */ _scrollToEndIfNeeded() { - const tokenCount = this.tokens.length; - if (this._lastTokenCount === undefined) { - this._lastTokenCount = tokenCount; - } - if (this._lastExpanded === undefined) { - this._lastExpanded = this.expanded; + // if token is focused skip scroll to the end + if (this._focusedToken) { + return; } - const tokenAdded = tokenCount > this._lastTokenCount; - const expandedNow = this.expanded && !this._lastExpanded; - - if (tokenAdded || expandedNow) { + if (this.tokens.length || this.expanded) { this.scrollToEnd(); } - - this._lastTokenCount = tokenCount; - this._lastExpanded = this.expanded; } _delete(e: CustomEvent) { @@ -916,13 +911,14 @@ class Tokenizer extends UI5Element { } _onfocusin(e: FocusEvent) { - const target = e.target as Token; this.open = false; - this._itemNav.setCurrentItem(target); + this.expanded = true; + this._addTokenToNavigation(e.target as Token); + } - if (!this.expanded) { - this.expanded = true; - } + _addTokenToNavigation(token: Token) { + this._focusedToken = token; + this._itemNav.setCurrentItem(token); } _onfocusout(e: FocusEvent) { @@ -933,6 +929,7 @@ class Tokenizer extends UI5Element { }); this._itemNav._currentIndex = -1; + this._focusedToken = undefined; this._skipTabIndex = true; if (!this.contains(relatedTarget)) { From 9fda1e983c59a5000715dbb7e367acc3666e14ac Mon Sep 17 00:00:00 2001 From: Milen Karmidzhanov Date: Thu, 11 Sep 2025 18:23:06 +0300 Subject: [PATCH 4/4] fix(ui5-tokenizer): fix scrolling bugs when expanding --- packages/main/src/MultiComboBox.ts | 3 +++ packages/main/src/MultiInput.ts | 5 +++++ packages/main/src/MultiInputTemplate.tsx | 1 + packages/main/src/Tokenizer.ts | 20 +++++++++++--------- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/main/src/MultiComboBox.ts b/packages/main/src/MultiComboBox.ts index 881dbfbffcc7..dd29fe884365 100644 --- a/packages/main/src/MultiComboBox.ts +++ b/packages/main/src/MultiComboBox.ts @@ -663,6 +663,7 @@ class MultiComboBox extends UI5Element implements IFormInputElement { _showFilteredItems() { this.filterSelected = true; this._showMorePressed = true; + this._tokenizer._scrollToEndOnExpand = true; this._toggleTokenizerPopover(); } @@ -1749,6 +1750,7 @@ class MultiComboBox extends UI5Element implements IFormInputElement { this._tokenizer.preventInitialFocus = true; if (this.open && !isPhone()) { + this._tokenizer._scrollToEndOnExpand = true; this._tokenizer.expanded = true; } @@ -1849,6 +1851,7 @@ class MultiComboBox extends UI5Element implements IFormInputElement { inputFocusIn(e: FocusEvent) { if (!isPhone()) { this.focused = true; + this._tokenizer._scrollToEndOnExpand = true; this._tokenizer.expanded = true; } else { this._innerInput.blur(); diff --git a/packages/main/src/MultiInput.ts b/packages/main/src/MultiInput.ts index e189997bc9ca..ed8ba51a93cd 100644 --- a/packages/main/src/MultiInput.ts +++ b/packages/main/src/MultiInput.ts @@ -209,6 +209,7 @@ class MultiInput extends Input implements IFormInputElement { } innerFocusIn() { + this.tokenizer._scrollToEndOnExpand = true; this.tokenizer.expanded = true; this.focused = true; @@ -217,6 +218,10 @@ class MultiInput extends Input implements IFormInputElement { }); } + _showMoreItemsPress() { + this.tokenizer._scrollToEndOnExpand = true; + } + _onkeydown(e: KeyboardEvent) { super._onkeydown(e); diff --git a/packages/main/src/MultiInputTemplate.tsx b/packages/main/src/MultiInputTemplate.tsx index d6ee8b24e41a..25bbe7cb0e00 100644 --- a/packages/main/src/MultiInputTemplate.tsx +++ b/packages/main/src/MultiInputTemplate.tsx @@ -27,6 +27,7 @@ function preContent(this: MultiInput) { onKeyDown={this._onTokenizerKeydown} onTokenDelete={this.tokenDelete} onFocusOut={this._tokenizerFocusOut} + onShowMoreItemsPress={this._showMoreItemsPress} > { this.tokens.map(token => )} diff --git a/packages/main/src/Tokenizer.ts b/packages/main/src/Tokenizer.ts index 0d9df43d7a09..aa38c2d02d5c 100644 --- a/packages/main/src/Tokenizer.ts +++ b/packages/main/src/Tokenizer.ts @@ -350,7 +350,11 @@ class Tokenizer extends UI5Element { _previousToken: Token | null = null; _focusedElementBeforeOpen?: HTMLElement | null; _deletedDialogItems!: Token[]; - _focusedToken?: Token; + /** + * Scroll to end when tokenizer is expanded + * @private + */ + _scrollToEndOnExpand = false; _handleResize() { this._nMoreCount = this.overflownTokens.length; @@ -408,7 +412,6 @@ class Tokenizer extends UI5Element { this._tokens.forEach(token => { token.forcedTabIndex = "-1"; - this._focusedToken = token; }); this._skipTabIndex = true; @@ -484,8 +487,8 @@ class Tokenizer extends UI5Element { * @protected */ _scrollToEndIfNeeded() { - // if token is focused skip scroll to the end - if (this._focusedToken) { + // if scroll to end is prevented, skip scroll to the end + if (!this._scrollToEndOnExpand) { return; } @@ -917,7 +920,7 @@ class Tokenizer extends UI5Element { } _addTokenToNavigation(token: Token) { - this._focusedToken = token; + this._scrollToEndOnExpand = false; this._itemNav.setCurrentItem(token); } @@ -929,7 +932,6 @@ class Tokenizer extends UI5Element { }); this._itemNav._currentIndex = -1; - this._focusedToken = undefined; this._skipTabIndex = true; if (!this.contains(relatedTarget)) { @@ -1034,7 +1036,7 @@ class Tokenizer extends UI5Element { const tokenRect = token.getBoundingClientRect(); const tokenContainerRect = this.contentDom.getBoundingClientRect(); - const oneSideBborderAndPaddingOffset = 5; + const oneSideBorderAndPaddingOffset = 5; const isLastToken = this._tokens.indexOf(token as Token) === this._tokens.length - 1; if (isLastToken) { @@ -1043,9 +1045,9 @@ class Tokenizer extends UI5Element { } if (tokenRect.left < tokenContainerRect.left) { - this._scrollEnablement?.scrollTo(this.contentDom.scrollLeft - (tokenContainerRect.left - tokenRect.left + oneSideBborderAndPaddingOffset), 0); + this._scrollEnablement?.scrollTo(this.contentDom.scrollLeft - (tokenContainerRect.left - tokenRect.left + oneSideBorderAndPaddingOffset), 0); } else if (tokenRect.right > tokenContainerRect.right) { - this._scrollEnablement?.scrollTo(this.contentDom.scrollLeft + (tokenRect.right - tokenContainerRect.right + oneSideBborderAndPaddingOffset), 0); + this._scrollEnablement?.scrollTo(this.contentDom.scrollLeft + (tokenRect.right - tokenContainerRect.right + oneSideBorderAndPaddingOffset), 0); } }