Nebular: [Menu] NbMenu - Deselect all menu items when this.findItemByUrl() returns undefined

[m-mo]

Issue type

I'm submitting a ... (check one with "x")

• [ ] bug report
• [x] feature request

Issue description

Current behavior:
When there are two NbMenu components active on a single page (for example, in the ngx-admin demo) and they both contain different NbMenuItems, both NbMenus will retain the .active class on the last NbMenuItem that was clicked, even if the router path does not match either of the active links.

Expected behavior:
I see that the NbMenu component subscribes to router event changes, during which a pathMatch full or partial check is initiated by the selectFromUrl method. However, the selectUrlFrom method will only set the selected property to true if there is a match. In the case when there is no match, it should de-select any active links.

Steps to reproduce:
1) Create two NbMenu components on a page, each being passed different NbMenuItem arrays.
2) Click on a link from the first NbMenu component, verify that the router has navigated to the link and that the link has the .active class.
3) Click on a link from the second NbMenu component, verify that the router has navigated to the link and both NbMenu components have .active classes on the links that were clicked.

Related code:
N/A

Other information:

npm, node, OS, Browser

Node: 10.13.0
OS: macOS High Sierra
Browser: Chrome 72.0

Angular, Nebular

Angular: 6.0
Nebular: 2.0.2

[aefox]

@m-mo thanks for raising this issue. it would help tremendously a stackblitz demonstrating the issue. this makes it faster for whomever will end up fixing this issue

[m-mo]

Hi @aefox please see the stackblitz below that reproduces the issue:
https://stackblitz.com/edit/ngx-admin-1845-hkegss

[aefox]

thanks a lot for that @m-mo !
@nnixaa this needs your input. do you think it would be ok to update this block here to do a reset of the selection, like:

if (selectedItem) {
  this.selectItem(selectedItem, items, collapseOther, tag);
} else {
  this.resetSelection(items);
}

I gave this a shot locally and it fixes the raised issue. But I am not sure if other scenarios would be affected or not

[yggg]

Thanks @m-mo and @aefox!
@aefox

do you think it would be ok to update this block here to do a reset of the selection, like:

Looks good to me. Since we don't have unselect event, we can ignore unselected items returned from resetSelection.

[n0mer]

@nnixaa @aefox are there any updates on this?