AppMenu/ModuleNav: accordion will cause double redirects

[LinweiW]

Describe the bug
When there is a guard in form page.
If the accordion is used, the icon will be clickable as well as the text area. If you only click the text area, the guard will be triggered and will do a refresh to another page. If the icon be clicked. it will skip the guard and redirect to the new page.
If the accordion is not used. The icon will not be clickable, the guard is able to prevent the page to redirect if you click the text area.
In WFM, it will cause the modal pops up twice. I think the problem is similar to the demo I made the difference is if the guard can catch the redirect twice.

To Reproduce
Steps to reproduce the behavior:

1. Go to https://stackblitz.com/edit/ids-quick-start-1401-6wlelz?file=src%2Fapp%2Fapp.component.html
2. Click on Form.
3. Then click on Home and Message in text area. And back to Form. Click the icon for Home and Message.
4. Switch between line 24 and line 25
5. Repeat step 2 and step 3.

Expected behavior
Icon and text area should be both clickable. The guard should able to prevent redirect. At least it won't cause double redirects.

Version

• ids-enterprise-ng: 13.3.2

Additional context
The guard setting is in form/form.component.ts. the checked boolean. switch between true and false.

[tmcconechy]

I tried to reproduce the issue but i dont understand the steps? What are the line numbers? Not following the example as it shows "works!" in all cases.

Do feel free to add a pull request if you see a fix.

[LinweiW]

line numbers are for app.component.html file. go into the app.component.html file. switch between
24.
25.

To be more specific, with line 24 used which accordion is not used, when you are in Form page, click Home. The deactivation modal will pop up. Close the modal and the menu(click on the white space in the content area). You will see that the page is not changed it's still in the Form page.

Then you comment out the line 24 and use line 25 in app.component.html file which accordion is used. When you are in Form page. Click Home, the deactivation modal will pops up. Close the modal, then the page will automatically do a refresh and still go to the Home page.
Also with line 25 used. When you are in Form page. Click Home's icon.

Then it will skip the guard goes to the Home page.

[tjamesallen15]

Hello @LinweiW,

During my investigation, I made some tweaks with your stackblitz example for it to work properly as intended.

For the deactivate-guard.ts, I put a parameter inside the injectable clause. This parameter made the blocking of redirection work properly

For the icons not triggering the redirection block, I notice that the routerLink is just inside the text.

So I changed the approach by putting the routerLink in the parent DIV

In this way, it will not bypass the guard clause.

cc @ericangeles and @tmcconechy

Stackblitz Link: https://stackblitz.com/edit/ids-quick-start-1401-7mexmu?file=src%2Fapp%2Fapp.component.html

Thanks!

[tmcconechy]

I think this seems like a good solution to me. Thanks @tjamesallen15 and it doesnt require changes

[edgarinfor]

@tmcconechy, @tjamesallen15 we did experiment before with moving the routerLink to the DIV and, even though it appeared to fix the problem other negative side effects occurred.

For example, Keyboard Navigation is broken, because when navigating with the Keyboard, the menu focuses on the A tag instead, not on the DIV even after adding routerLink to the DIV.

The IDS component sort of depends on the A tag which with this "workaround" doesn't make sense anymore. Also, the SVG should have been inside the A tag to begin with in order to make it "clickable" as well.

Also, even if the <a>Home</a> is "valid" mark-up, it is semantically meaningless at this point and breaks accessibility (which may required adding extra "aria" attributes).

If the a element has no href attribute, then the element represents a placeholder for where a link might otherwise have been placed, if it had been relevant, consisting of just the element's contents.

So, for this solution to be satisfactory, we need IDS to adopt this "suggested structure" and make sure (QA) that the menu and derived functionality work, otherwise, it is not much different from us "hacking" it, which is what we are trying to avoid.

[LinweiW]

Hi team, follow up. Is it possible to have a solution to avoid scenarios that @edgarinfor left?

[tmcconechy]

@ericangeles are you looking at this one? For me i really have no clue how we could solve it. Since we dont manage the router itself. Its sort of up to the implementer in my opinion. Unless we add an example and show the "right way" which is what @tjamesallen15 seems to do.

I upped the estimate to the highest level of complexity. Maybe @edgarinfor has a concrete fix he could do a pull request for?

[edgarinfor]

@tmcconechy , based on my previous comment, the proper solution should begin by revisiting the structure of the Component's Template because that is part of the problem, or at least, fix the keyboard navigation so that when putting the Router Link on the parent DIV, it works.

Regarding the Router, this is a Navigation component of an Angular library, and the Router is the official Angular feature for Navigation so, the library should support it properly. This is not an implementation issue, we already tried it on our end.

However, I may agree with the level of complexity but, maybe we already have enough information to approach the solution. @LinweiW , would you guys have the capacity to attempt/contribute a fix for the library?

[LinweiW]

@edgarinfor Unfortunately no, we are working on the new features.

[ericangeles]

@tmcconechy we’ll take a look at it. Let’s sync up and do an offline discussion, @tjamesallen15.

[edgarinfor]

Is it possible to double check if this issue also occurs with the new module-nav-bar?

We have been just made aware that there is interest to replace the soho-application-menu.

Both components use the accordion as well so, maybe the issue affects both but, we haven't tested it yet.

[tmcconechy]

Ok lets us know what you find @edgarinfor ~ we will try the same when we get to this. And maybe fix only for module nav.

[tmcconechy]

@LinweiW we dont have any solutions for this one. Closing it for now unless you have some ideas?