Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Menu | itemClick requires double click in popup mode if items generated by function #13934

Closed
roman-bychkov opened this issue Oct 20, 2023 · 18 comments
Assignees
Labels
LTS-PORTABLE Issue's fix will be ported to supported LTS versions Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@roman-bychkov
Copy link

roman-bychkov commented Oct 20, 2023

Describe the bug

Component: Menu inside a table row.
User should click twice on menu action to reach an action.

Environment

  1. Click on Menu in table row
  2. Click on Menu action once (will nothing happen)
  3. Click on Menu action again - Action will executed

Reproducer

https://stackblitz.com/edit/8numcb-pnb4qf?file=src%2Fapp%2Fdemo%2Ftable-basic-demo.html

Angular version

16.2.0

PrimeNG version

16.5.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

v18.12.1

Browser(s)

No response

Steps to reproduce the behavior

  1. Click on Menu in table row
  2. Click on Menu action (nothing happens)
  3. Click on Menu action again - Action will executed

Expected behavior

When user click first time on menu action. Action should be execute.

@roman-bychkov roman-bychkov added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Oct 20, 2023
@roman-bychkov roman-bychkov changed the title Component: Menu inside a table row. You need to click twice to reach action Component: Menu inside a table row. You need to click twice to reach an action Oct 20, 2023
@roman-bychkov
Copy link
Author

any suggestions?

@dnordahl1
Copy link

dnordahl1 commented Nov 28, 2023

In fiddling with this trying to find a workaround, I noticed that the bug goes away if you set an invalid appendTo, but not if you leave off appendTo. In comparing the menu objects after toggling, I noticed the ones that don't work (those without invalid appendTo), have a documentClickListener defined that points to a function called removeEventListener. Whereas the menu with the invalid appendTo does not have this. Also after the MenuItem command is executed, I notice none of the menus have this documentClickListener set.

Stackblitz example: https://stackblitz.com/edit/8numcb-2leua7

I'm not sure how to formulate a workaround based on this, or if it has anything to do with the issue. I tried removing the documentClickListener in the toggle event to see if that works without luck, but I'm not sure I'm doing so correctly. Perhaps this documentClickListener needing to be consumed is what the first/ineffective click on a menu item is accomplishing. After which point the second click is then able to work?

@cetincakiroglu cetincakiroglu added Type: Bug Issue contains a bug related to a specific component. Something about the component is not working and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Dec 26, 2023
@cetincakiroglu cetincakiroglu added this to the 17.3.0 milestone Dec 26, 2023
@cetincakiroglu cetincakiroglu self-assigned this Dec 26, 2023
@cetincakiroglu cetincakiroglu added the LTS-PORTABLE Issue's fix will be ported to supported LTS versions label Dec 26, 2023
@cetincakiroglu
Copy link
Contributor

Hi,

Thanks a lot for reporting the issue, great catch!

@cetincakiroglu cetincakiroglu changed the title Component: Menu inside a table row. You need to click twice to reach an action Menu | itemClick requires double click in popup mode Dec 27, 2023
@cetincakiroglu cetincakiroglu changed the title Menu | itemClick requires double click in popup mode Menu | itemClick requires double click in popup mode if items generated by function Dec 27, 2023
@cetincakiroglu
Copy link
Contributor

cetincakiroglu commented Dec 28, 2023

Hi,

After investigating the matter, I've discovered an interesting correlation with the tabindex attribute within the <ul></ul> element. As part of our accessibility enhancements, we've incorporated a tabindex value along with focus and blur events to facilitate menu accessibility. However, this implementation seems to trigger the issue specifically when options are provided by a function. I tested your code in versions v14, v15, and v16 (pre-accessibility) and observed that the issue did not occur previously because the menu lacked tabindex input and support for focus and blur. Introducing a tabindex to the top ul element caused the issue, necessitating a double click. At present, the most effective solution seems to be setting tabindex to undefined. I've added a method to set tabindex to 'undefined'.

cetincakiroglu added a commit that referenced this issue Dec 28, 2023
@dnordahl1
Copy link

@cetincakiroglu Do you know if this fix will be added to a version 16 release after being verified in 17?

In the stackblitz example I tried setting tabindex to undefined, null, and a value in both the menuItem html attributes and the model attributes, and it didn't seem to provide a workaround.

@cetincakiroglu
Copy link
Contributor

Try this

<p-menu [tabindex]="undefined"></p-menu>

@dnordahl1
Copy link

@cetincakiroglu That doesn't seem to work either:

https://stackblitz.com/edit/8numcb-56sipg

@cetincakiroglu
Copy link
Contributor

@dnordahl1
Copy link

@cetincakiroglu I'm hoping for a work around for version 16 so that we're able to be on an LTS release.

@IvanNovikov-mobdev
Copy link

@dnordahl1 I have implemented this workaround:

https://stackblitz.com/edit/8numcb-tc1ytv?file=src%2Fapp%2Fdemo%2Ftable-basic-demo.html,src%2Fapp%2Fdemo%2Ftable-basic-demo.ts

Hope this helps someone :)

@dnordahl1
Copy link

@IvanNovikov-mobdev Excellent.. I'm very grateful for you sharing this!

@dorthrithil
Copy link
Contributor

@cetincakiroglu This is still an issue in 17.9.0
[tabindex]="undefined" didn't help.

@cetincakiroglu
Copy link
Contributor

@cetincakiroglu This is still an issue in 17.9.0 [tabindex]="undefined" didn't help.

Hi,

Thanks for letting us know, we'll check before 17.13.0 release.

@clairebones
Copy link

Is it known if this is fixed in 17.18.4 before we upgrade to it? We're been stuck on 15 because we can't afford to introduce this issue.

@liefra
Copy link

liefra commented Aug 18, 2024

The workaround [tabindex]="undefined" works for me using version 17.18.9

@Spiff77
Copy link

Spiff77 commented Sep 23, 2024

Hello,

For some reason i am still facing the problem with 17.18.9 and 17.18.10

<div class="card flex justify-content-center">
  <p-menu #menu
          [tabindex]="undefined" styleClass="z-2" [model]="getMenuItemsForItem(item)" [popup]="true" appendTo="body">
    <ng-template pTemplate="item" let-mitem>
      @if (mitem.route) {
        <a [routerLink]="mitem.route" class="p-menuitem-link">
          <span [class]="mitem.icon"></span>{{mitem.route+''+item.id}}
          <span class="ml-2">{{ mitem.label }}</span>
        </a>
      } @else {
        <a class="p-menuitem-link">
          <span [class]="mitem.icon"></span>
          <span class="ml-2">{{ mitem.label }}</span>
        </a>
      }
    </ng-template>
  </p-menu>
  <p-button [rounded]="true" [outlined]="true" (onClick)="menu.toggle($event)" icon="pi pi-ellipsis-v"/>
</div>

with this in ts

getMenuItemsForItem(item: Survey): MenuItem[] {
    return [
      {
        label: 'Options',
        items: [
          {
            label: 'Edit',
            icon: 'pi pi-file-edit',
            route: '/survey/add/'+item.id,
          },
          {
            label: 'Manage answers',
            icon: 'pi pi-chart-bar',
            route: '/participation/survey/'+item.id,
          },
          {separator: true},
          {
            label: 'Delete',
            icon: 'pi pi-trash',
            style: {'color': 'red'},
            command: () => {
              this.deleteCl(item.id)
            }
          }
        ]
      }
    ]

@AlbaSS18
Copy link

The issue is present in v.17.18. I have set [tabindex]="undefined" and have worked for me. In my case, i have to override p-button for button.

@devmuzaky
Copy link

The issue is present in v.17.18. I have set [tabindex]="undefined" and have worked for me. In my case, i have to override p-button for button.

thank you so much, works with me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTS-PORTABLE Issue's fix will be ported to supported LTS versions Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

10 participants