feat(combobox): display overflowing selected content in a tooltip #9014
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -481,6 +481,7 @@ export class Combobox | |
| disconnectedCallback(): void { | ||
| this.mutationObserver?.disconnect(); | ||
| this.resizeObserver?.disconnect(); | ||
| this.textLabelElResizeObserver?.disconnect(); | ||
| disconnectInteractive(this); | ||
| disconnectLabel(this); | ||
| disconnectForm(this); | ||
|
|
@@ -531,6 +532,9 @@ export class Combobox | |
|
|
||
| @State() text = ""; | ||
|
|
||
| /** keeps track of the tooltipText */ | ||
| @State() tooltipText: string; | ||
|
|
||
| /** when search text is cleared, reset active to */ | ||
| @Watch("text") | ||
| textHandler(): void { | ||
|
|
@@ -557,6 +561,9 @@ export class Combobox | |
| this.refreshSelectionDisplay(); | ||
| }); | ||
|
|
||
| /** keep track of the rendered textLabelEl */ | ||
| private textLabelEl: HTMLSpanElement; | ||
|
|
||
| private guid = guid(); | ||
|
|
||
| private inputHeight = 0; | ||
|
|
@@ -764,6 +771,31 @@ export class Combobox | |
| this.el.removeEventListener("calciteComboboxOpen", this.toggleOpenEnd); | ||
| }; | ||
|
|
||
| private setTooltipText = (): void => { | ||
| const { textLabelEl } = this; | ||
| if (!textLabelEl) { | ||
| return; | ||
| } | ||
|
|
||
| requestAnimationFrame(() => { | ||
| this.tooltipText = | ||
| textLabelEl.offsetWidth < textLabelEl.scrollWidth ? textLabelEl.innerText : null; | ||
|
There was a problem hiding this comment. I would compare the actual text width using this util with the computed width of the text's container. Also, rather than making a separate There was a problem hiding this comment. I don't think that util makes sense in this scenario. We just want to know if the offsetWidth is less than the scrollWidth which is simpler than creating a canvas and measuring. I do agree that the There was a problem hiding this comment. Note: Button seems to do the same logic so a util that handles this might be useful. Something like There was a problem hiding this comment. I also thought, why not just always render the full There was a problem hiding this comment. Yeah I thought that too. IMO that would be the simplest option. It is a tiny bit odd to conditionally render a title attribute. There was a problem hiding this comment. This came up with tab-title/dropdown truncation as well. Since truncation is happening on all long text items, we decided at that time that there was no need to have a popup with duplicate info where not necessary. The question was, what would a dropdown item look like with a title tooltip with a repeat text and calcite-tooltip? There was a problem hiding this comment. Thanks @Elijbet for the refresher! If we play this out to possible truncation behavior in places like List or Tree, I agree that keeping this conditional is a cleaner UX for the end user. If the text is short enough to be displayed then there isn't much reason to also render the There was a problem hiding this comment. We've also been talking about mapping out a system-wide configurable There was a problem hiding this comment. Ok, that makes sense to me. There was a problem hiding this comment. Closing the issue then. |
||
| }); | ||
| }; | ||
|
|
||
| private textLabelElResizeObserver = createObserver("resize", this.setTooltipText); | ||
|
|
||
| private setTextLabelEl = (el: HTMLSpanElement): void => { | ||
| this.textLabelEl = el; | ||
| this.setTooltipText(); | ||
| }; | ||
|
|
||
| private setTextInputWrapEl = (el: HTMLInputElement): void => { | ||
| this.textInput = el; | ||
| this.textLabelElResizeObserver?.observe(el); | ||
| this.setTooltipText(); | ||
| }; | ||
|
|
||
| onBeforeOpen(): void { | ||
| this.scrollToActiveItem(); | ||
| this.calciteComboboxBeforeOpen.emit(); | ||
|
|
@@ -1527,17 +1559,20 @@ export class Combobox | |
| } | ||
|
|
||
| private renderInput(): VNode { | ||
| const { guid, disabled, placeholder, selectionMode, selectedItems, open, tooltipText } = this; | ||
| const single = isSingleLike(selectionMode); | ||
| const selectedItem = selectedItems[0]; | ||
| const showLabel = !open && single && !!selectedItem; | ||
|
|
||
| return ( | ||
| <span | ||
| class={{ | ||
| [CSS.inputWrap]: true, | ||
| [CSS.inputWrapSingle]: single, | ||
| }} | ||
| title={tooltipText} | ||
| // eslint-disable-next-line react/jsx-sort-props -- ref should be last so node attrs/props are in sync (see https://github.com/Esri/calcite-design-system/pull/6530) | ||
| ref={this.setTextInputWrapEl} | ||
| > | ||
| {showLabel && ( | ||
| <span | ||
|
|
@@ -1546,6 +1581,8 @@ export class Combobox | |
| "label--icon": !!selectedItem?.icon, | ||
| }} | ||
| key="label" | ||
| // eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop | ||
| ref={this.setTextLabelEl} | ||
| > | ||
| {selectedItem.textLabel} | ||
| </span> | ||
|
|
@@ -1688,6 +1725,7 @@ export class Combobox | |
| [CSS.selectionDisplaySingle]: singleSelectionDisplay, | ||
| }} | ||
| key="grid" | ||
| // eslint-disable-next-line react/jsx-sort-props -- auto-generated by @esri/calcite-components/enforce-ref-last-prop | ||
| ref={this.setChipContainerEl} | ||
| > | ||
| {!singleSelectionMode && !singleSelectionDisplay && this.renderChips()} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this should just change to
@State() showTooltipText = false;No need to store the string value since its just the selected item's display value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Storing the value is duplicative. You should be able to get the selected item's value with the element reference in the ResizeObserver callback function.