Skip to content

Commit

Permalink
workaround #47555
Browse files Browse the repository at this point in the history
  • Loading branch information
bpasero committed Jun 11, 2018
1 parent 5695c53 commit 68f1bc2
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/vs/base/browser/ui/selectBox/selectBoxCustom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,10 @@ export class SelectBoxList implements ISelectBoxDelegate, IDelegate<ISelectOptio
this.selectList.domFocus();

// Finally set focus on selected item
this.selectList.setFocus([this.selected]);
this.selectList.reveal(this.selectList.getFocus()[0]);
if (this.selectList.length > 0) {
this.selectList.setFocus([this.selected || 0]);
this.selectList.reveal(this.selectList.getFocus()[0] || 0);
}

// Set final container height after adjustments
this.selectDropDownContainer.style.height = (listHeight + totalVerticalListPadding) + 'px';
Expand Down

2 comments on commit 68f1bc2

@cleidigh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joaomoreno
@bpasero
Unfortunately I have had to be AWOL for several weeks. I missed this assignment, apologies, thanks for taking care of this.

From looking at this previously I wonder about a couple issues still:

  • If there are no options should we return from showSelectDropdown without expanding the drop-down?
  • If there are no options should we have a title placeholder if the drop-down is rendered? e.g. "no options"

looking to get back on the contribution horse ;-)

@bpasero
Copy link
Member Author

@bpasero bpasero commented on 68f1bc2 Jun 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cleidigh no worries, take your time.

If there are no options should we return from showSelectDropdown without expanding the drop-down?

Yes I think so, currently it looks a bit weird in that case.

If there are no options should we have a title placeholder if the drop-down is rendered? e.g. "no options"

Not sure, I think just not expanding might be fine enough.

Please sign in to comment.