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

Notification drawer example dropdown kebab broken #759

Closed
skateman opened this issue Aug 17, 2018 · 11 comments
Closed

Notification drawer example dropdown kebab broken #759

skateman opened this issue Aug 17, 2018 · 11 comments
Labels

Comments

@skateman
Copy link
Member

skateman commented Aug 17, 2018

I was playing with the notification drawer example and found two issues, in notification-body.html:

  • The dropdownKebabRight ID is being used in the repeated region, i.e. possibly generates duplicate IDs
  • The data-toggle="dropdown" attribute on the <button> conflicts with the JS provided by bootstrap. This causes the kebab to activate after a second click only.
@skateman skateman added the bug label Aug 17, 2018
@dtaylor113
Copy link
Member

dtaylor113 commented Aug 17, 2018

...This causes the kebab to activate after a second click only.

Hi @skateman, sorry, I'm not seeing this behavior? The kebab opens on the first click for me (using Chrome)?
image

@skateman
Copy link
Member Author

skateman commented Aug 17, 2018

@dtaylor113 oh, you're probably not including the original bootstrap JS on the page. I spend an hour with @Hyperkid123 when we found why it wasn't working in MiQ 😆

@dtaylor113
Copy link
Member

Hi @skateman, ok I removed data-toggle="dropdown" from the <button/> in notification-body.html for the Notification Drawer ngDoc example and the kabob menu continues to work on a single click.
<button/> in notification-body.html still has uib-dropdown-toggle which I believe is the correct angular-ui bootstrap attribute.

I'll work on dropdownKebabRight ID issue next.
Thanks,
Dave

@skateman
Copy link
Member Author

@dtaylor113 yes, the uib-dropdown-toggle is okay, I fixed the attribute to the data-toggle above, sorry and thanks

@dtaylor113
Copy link
Member

I fixed the attribute to the data-toggle above, sorry

Sorry, where did you fix it?

@skateman
Copy link
Member Author

@dtaylor113 in the PR description I had dropdown-toggle instead of data-toggle="dropdown"

@dtaylor113
Copy link
Member

Hi @skateman, do you need id="dropdownKebabRight"? I removed it from notification drawer example, and the example and our unit tests continue to work correctly.

@skateman
Copy link
Member Author

Not sure about the aria- prefix on the dropdown content, it has probably some semantic value. Ideally here you would have to generate a dynamic ID for each notification as we do it here. I know it's not pretty, but it would be nice if we could prevent people from copy-pasting bad stuff (we have a bunch of copy-pasted PF examples that cause duplicate ID issues and that makes QE cry sometimes).

@dtaylor113
Copy link
Member

I'm still not sure that example generates a unique id across all kabob action dropdowns. It's based on notification.id, where it should be something like 'dropdownKebabRight-{{ notification.id }}-{{ notification.action.id }}' because there may be mutliple actions per notification.

@skateman
Copy link
Member Author

Yup, but only one dropdown per-notification and the ID belongs to the dropdown.

@dtaylor113
Copy link
Member

ah, yes, correct, it's been a while :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants