Selectbox breaks when there's not enough space to render

[joaomoreno]

1. Open the Debug viewlet
2. Open the No Configurations select box, all is good
3. Resize window to minimum height
4. Open the select box again, seems broken
5. Resize window to regular height
6. Select box stays broken

[cleidigh]

@joaomoreno
Thanks, on it...

Just for my curiosity, I assume you forced the use of the custom select box on the Mac platform?
I reproduced it in Windows, probably my vertical flip calculations

[cleidigh]

@joaomoreno
@bpasero

I have fixed the calculations and added logic to not open the drop-down at all if there is insufficient room below or above. I do need a recommendation on the final part:

I can't figure out a best practice to hide/close the base contextview for the sequence where the drop-down is already open , window or region is resized, Global layout is called eventually getting to the contextview. If I attempt to hide from within the layout call Code crashes since the delegate is destroyed within the layout call.

Doing a CSS hide within the delegate layout causes a blur event where I currently do a hide . This sequence only works if the blur event occurs after the layout returns. I don't think this can be guaranteed so I don't like it is a solution.

I looked at the feedback code to see what it does but I cannot find any parallel.

The ability to Close a contextview from within the layout method or rather the delegate , would seem to be useful but currently not implemented?

Thanks

[bpasero]

@joaomoreno can maybe advice on that question around the context view given his recent stake in that widget.

[joaomoreno]

@cleidigh Can you show me the spot where you'd attempt to hide the context view during "Global layout"?

[cleidigh]

@joaomoreno

Below is the beginning part of the delegate layout function for the contextview based
custom select box. My first solution, listed as Approach 1 in the comments, calls
contextview.hide() if nothing more fit above or below. The call is made within
the delegate layout method. This crashes because the contextview.doLayout private method
is called after the delegate and after inspection does not check if the delegates still exist
as elsewhere.

After some experimenting, I have a pretty basic fix but, it requires a change to contextview
which I was hoping and assuming I could avoid.

I also include the patch at the end. I posted a WIP PR with this working approach.
Please ignore the sequencing debug output and of course it needs to be cleaned up and finished.
PR #55930

If you agree with the fix, let me know when I should merge (for July or August)?

selectBoxCustom.ts

	private layoutSelectDropDown(preLayoutPosition?: boolean): boolean {

		// Layout ContextView drop down select list and container
		// Have to manage our vertical overflow, sizing, position below or above
		// Position has to be determined and set prior to contextView instantiation

		console.debug('Layout '+ preLayoutPosition);
		if (this.selectList) {

			const selectPosition = dom.getDomNodePagePosition(this.selectElement);
			const styles = getComputedStyle(this.selectElement);
			const verticalPadding = parseFloat(styles.getPropertyValue('--dropdown-padding-top')) + parseFloat(styles.getPropertyValue('--dropdown-padding-bottom'));
			let maxSelectDropDownHeightBelow = 0;
			maxSelectDropDownHeightBelow = (window.innerHeight - selectPosition.top - selectPosition.height - this.selectBoxOptions.minBottomMargin);
			const maxSelectDropDownHeightAbove = (selectPosition.top - SelectBoxList.DEFAULT_DROPDOWN_MINIMUM_TOP_MARGIN);

			this.selectList.layout();
			let listHeight = this.selectList.contentHeight;
			const minRequiredDropDownHeight = listHeight + verticalPadding;

			if ( (maxSelectDropDownHeightBelow + maxSelectDropDownHeightAbove) < this.getHeight() + verticalPadding) {
				// If at least one option cannot be shown, don't open the drop-down or hide/remove if open
				if (this._isVisible) {
					// We must be doing a re-layout, hide if we don't fit

					// Approach 1: ContextView.hide - fails when ContextView.doLayout called
					
					this.hideSelectDropDown(true);
					
					// Above approach will work with minimal change in ContextView.layout
					// See following code block


					// Approach 2: CSS hide triggers this.selectList.onDidBlur which destroys ContextView
					// as long as blur occurs after ContextView.layout, hide/remove call succeeds
					// I don't think this can be guaranteed, but I have not seen it fail
					// dom.toggleClass(this.selectDropDownContainer, 'visible', false);

					// Approach 3: Focus back on parent select, causes immediate blur
					// and hide before layout returns, always fails
					// This would also be the failure mode for approach 2
					// this.selectElement.focus();
				}
				return false;
			}

contextview.ts patch: Added delegate existence check

private doLayout(): void {

		// Check that we still have a delegate - this.delegate.layout may have hidden
		if (!this.isVisible()) {
			return;
		}

		// Get anchor
		let anchor = this.delegate.getAnchor();

[joaomoreno]

@cleidigh If I understand correctly, you simply chose not to open the widget in case there is no available space. But the widget supports scrollbars... it should open correctly...

[cleidigh]

@joaomoreno
You are correct in the case the drop down is already open. That scenarios taken care of. Dropdown will remain closed and the user can change with the keyboard.
The issue you originally posted where the drop-down is already open and the window is resized or zoomed (which I do a lot) and eventually there is no room. it will had scrollbars in between. In this case
the delegate layout has to make the determination when called for relayout

The patch I proposed allows the delegate layout to determine if there is space available and close the contextV if necessary. Since the check is the same if canReLayout === false I don't see any problem taking this approach.

[joaomoreno]

Actually, my original issue is not about resizing while the box is open, but rather about opening the box while there's not enough space for it to render. Check the GIF.

[cleidigh]

@joaomoreno
I see I poorly conflated and confused the two issues in my last comment...

Original issue is solved by first patch to selectBoxCustom.ts above preventing opening if insufficient space to render.

Second issue (and second fix) was identified while fixing the first. DropDown should close if already open, insufficient space to render on DOM re-layout.

Should I create a separate issue/PR fix for the second "SelectBox should close on DOM re-layout if insufficient space to render" ?

[joaomoreno]

Original issue is solved by first patch to selectBoxCustom.ts above preventing opening if insufficient space to render.

Let me go back to my point: the widget uses a list which supports scrollbars. So, shouldn't we still open it, instead of preventing it from opening, since the scrollbars will take care of the content overflow?

[cleidigh]

@joaomoreno
The scenario to not open is when the layout calculates that it is not possible to fit even a single option
below or above and we do not want to show an empty box with a border. I can only prevent this
by hiding and refocusing the select.

As long as we can show one option below or above if there are more options the scrollbars will be visible.

I will clean up the PR so there's no extraneous stuff or debug output so the two patches are not muddied

Update: Found another edge condition under settings editor: select gets moved out of viewport
have to hide in that case also. will post PR tomorrow.