Skip to content
This repository has been archived by the owner on Sep 1, 2020. It is now read-only.

fix issue 168: Menu doesn't close when press 'Enter' #170

Merged
merged 1 commit into from
Nov 29, 2017

Conversation

WaterDesk
Copy link
Contributor

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

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) {
Copy link
Owner

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,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not exactly, I think. Below is the console output of this.selectedItemRef:
screen shot 2017-11-29 at 1 56 01 pm

Copy link
Owner

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??

Copy link
Contributor Author

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;

screen shot 2017-11-29 at 2 08 36 pm

Copy link
Contributor Author

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

Copy link
Contributor Author

@WaterDesk WaterDesk Dec 1, 2017

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.

Copy link
Owner

Choose a reason for hiding this comment

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

@WaterDesk no issues

@vkbansal vkbansal merged commit 43996c0 into vkbansal:master Nov 29, 2017
@WaterDesk
Copy link
Contributor Author

Thank you to merge the code. Could you please help to deliver a release 2.8.2?

@vkbansal
Copy link
Owner

Sure, will do it in a day or two

@zhouzhongyuan zhouzhongyuan mentioned this pull request Nov 29, 2017
6 tasks
@WaterDesk WaterDesk deleted the Issue-168 branch November 30, 2017 07:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants