Add option the popup to close on mouseleave

[darkred]

Closes #123

Test URLs

https://github.com/

Screenshot

---

Note to reviewers

I use .removeAttribute('open') to close the dropdown.

[fregante]

Thank you! I’ll make sure it works correctly soon

[darkred]

I made a change so that the popup close gets canceled if you re-mousenter quick enough, within 1 sec (1000 ms) (and updated the screenshot in the OP).

[darkred]

Hi @fregante Is there any way I can assist you in making the PR complete ?

  Some notes:

• About the popup closing to be canceled if you re-mousenter quick enough, the interval I currently have is 1000 msec, but I think it should be 500 msec. What do you think?

• If I click a "Mark as done" and then move the mouse away so that the popup gets closed, then if I move the mouse to reopen the popup, clicking another "Mark as done" unfortunately causes the relevant "Marked x notifications as read" message to be displayed multiple times: e.g. So, I tried changing removeAttribute() into remove() the popup altogether + reopening it (see code below) but unfortunately the issue remains:

  using remove() :

  ```js if (options.closeOnMouseleave) { let timer; select('.NPG-dropdown').addEventListener('mouseleave', () => { // timer = setTimeout(closeDropdown, 500); timer = setTimeout(() => {closeDropdown(); createNotificationsDropdown();}, 500); }); select('.NPG-dropdown').addEventListener('mouseenter', () => { clearTimeout(timer); }); } // ... function closeDropdown() { // select('.NPG-container[open]').removeAttribute('open'); select('.NPG-container').remove(); } ```

[darkred]

Thank you! I’ll make sure it works correctly soon

@fregante Have you changed your mind? Do you find this PR is not salvageable? Should I close this?

[fregante]

I've been on Safari for a year and I haven't used some of my extensions since 😅 so you can just leave this open until someone reviews it

[tjx666]

Welcome to contribute here: https://github.com/tjx666/github-notifications-preview/tree/main

[darkred]

@fregante Thanks for the merge. Just wanted to note, though, that the problem with my code that I reported above still exists (the additional multiple "Marked 0 notifications as read").

Screenshot:


  Is it ok if I open an issue for reference? (I haven't managed to fix this problem myself). Or, it's better to revert the merge altogether, so someone else can provide a fully working PR?

[fregante]

If this PR introduced the bug, just reuse the closing logic that we already had: select('.modal-backdrop').click();

You can place this line in closeDropdown and replace the old select('.modal-backdrop').click(); with closeDropdown() as well.

Note that we're still waiting for this to be released though

[darkred]

Thanks a lot for the guidance. So, I tried your suggestions but I noticed that the .modal-backdrop selector no longer applied (you had used it 7 years ago here). If you could provide me with a working one, I'd gladly make a new PR. (I apologize for asking for your help once again, but I can't seem to find a working one myself).

Note that we're still waiting for this to be released though

For Firefox it was released alright.

[fregante]

Check how it's already working. Clicking somewhere manually closes the dropdown, so you can see what the new selector is.

[darkred]

The selector I've found is details.NPG-container[open] > summary and it works, but unfortunately the duplicate additional multiple "Marked 0 notifications as read") still occur. Here are the 2 changes in total:

and here is a screen capture of the issue:


[fregante]

It's not an issue with this PR. It can be fixed separately:

• https://github.com/tanmayrajani/notifications-preview-github/issues/107

PR welcome with the change I suggested anyway

[fregante]

I just submitted this version to the chrome web store. They should publish it within 24 hours