-
Notifications
You must be signed in to change notification settings - Fork 376
fix issue 168: Menu doesn't close when press 'Enter' #170
Conversation
There are two cases: 1. Given the menu is open and the selected item is disabled, When I press Enter, the menu doesn't close, the expected behavior is that the menu should close 2. Given the menu is open and no item is selected, When I press Enter, the menu doesn't close, the expected behavior is that the menu should close
if (this.seletedItemRef && this.seletedItemRef.ref instanceof HTMLElement) { | ||
if (this.seletedItemRef && | ||
this.seletedItemRef.ref instanceof HTMLElement && | ||
!this.seletedItemRef.ref.props.disabled) { |
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.
AFAIK if this.seletedItemRef.ref instanceof HTMLElement
is true
, this.seletedItemRef.ref.props
will not exist since it will not refer to a ReactElement
. Correct me if I'm wrong,
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.
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.
Can you please, also show the output for this.seletedItemRef.ref instanceof HTMLElement
??
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.
case 13:
// enter
e.preventDefault();
_this2.tryToOpenSubMenu(e);
console.log("handleKeyNavigation es6");
console.log(_this2.seletedItemRef);
if (_this2.seletedItemRef) {
console.log("_this2.seletedItemRef.ref instanceof HTMLElement");
console.log(_this2.seletedItemRef.ref instanceof HTMLElement);
}
if (
_this2.seletedItemRef &&
_this2.seletedItemRef.ref instanceof HTMLElement &&
!_this2.seletedItemRef.props.disabled
) {
_this2.seletedItemRef.ref.click();
} else {
console.log("hideMenu");
_this2.hideMenu(e);
}
break;
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.
this.seletedItemRef.ref instanceof HTMLElement
is true
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.
@vkbansal I am afraid I made a mistake in this PR. The code change in my local to determine the disabled status of the menuitem is !_this2.seletedItemRef.props.disabled
. But there was a typo in the PR: !this.seletedItemRef.ref.props.disabled
. I will create another PR. Very sorry for my mistake.
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.
@WaterDesk no issues
Thank you to merge the code. Could you please help to deliver a release 2.8.2? |
Sure, will do it in a day or two |
There are two cases: