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

Revert "6318: fixed mention complete with mouse clicking" #6795

Merged

Conversation

marcoambrosini
Copy link
Member

@marcoambrosini marcoambrosini commented Jan 19, 2022

Reverts #6789

Fix #6318

@marcoambrosini
Copy link
Member Author

/backport to stable23

Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works 👍

However, @marcoambrosini when reverting please describe why you are reverting :-) In this case I guess it is due to the event listener being set again and again when the input is focused, as it uses an arrow function, but better to have it explicit in the pull request description.

Could you also explain why the original code (before #6789) failed and why your change fixes it? I have tested just a few times and it worked, but it would be good to know where was the problem to test it in more detail, as the original code also worked some times.

Similarly, @nikola-gladovic, could you explain why your code fixed the issue too so both approaches can be compared?

Thanks :)

@marcoambrosini
Copy link
Member Author

@danxuliu the commit message explanation is not good enough?? xD

Increased the timeout in order to let the vue-at componentdo its magic before destroying it after a blur event on the advanced input.

Signed-off-by: marco <marcoambrosini@pm.me>
@marcoambrosini marcoambrosini force-pushed the revert-6789-bugfix/6318/mention-complete-with-mouse branch from 6b456cf to 4dd62b5 Compare January 20, 2022 10:17
@danxuliu
Copy link
Member

danxuliu commented Jan 20, 2022

@danxuliu the commit message explanation is not good enough?? xD

Ah, sorry, I should have looked there, indeed ;-)

Relying on timeouts to do something is flaky, though... It would be interesting to understand why Nikola's approach also fixed the issue, as it could be a safer approach once the duplicated listener is fixed 🤔

@nickvergessen
Copy link
Member

Relying on timeouts to do something is flaky, though... It would be interesting to understand why Nikola's approach also fixed the issue, as it could be a safer approach once the duplicated listener is fixed thinking

But that is what the lib author mentions as solution:
fritx/vue-at#114 (comment)

@nickvergessen nickvergessen merged commit fa627fe into master Jan 26, 2022
@nickvergessen nickvergessen deleted the revert-6789-bugfix/6318/mention-complete-with-mouse branch January 26, 2022 14:37
@nickvergessen
Copy link
Member

/backport 4dd62b5 to stable23

@backportbot-nextcloud
Copy link

The backport to stable23 failed. Please do this backport manually.

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

Successfully merging this pull request may close these issues.

Can not complete a mention with the mouse
3 participants