Less intrusive sharp popup (reference suggestion/autocomplete)

[MarcelRobitaille]

Fixes #903

I'm creating a pull request so we can keep track of the remaining tasks for this issue in this first comment:

Generalization:

• It currently only works on EditInputGroup. To prevent duplicating code, consider Vue mixin.

Responsiveness:

• Ensure the popup does not go off screen if the # appears in the right of the input.
• Make the popup width: 100%, left: 0 for small screens.
• The popup should have a maximum height. If this maximum height is reached, it should be possible to scroll with the mouse wheel AND by navigating through the options with up and down arrow keys.

I'm not sure if you can edit this comment. If not, if there are any other to-do items, please leave a comment and I will update this list.

[github-actions]

Unit Test Results

     33 files       33 suites   7m 0s ⏱️
   403 tests    403 ✔️ 0 💤 0 ❌
4 433 runs  4 433 ✔️ 0 💤 0 ❌

Results for commit 92d6349.

♻️ This comment has been updated with latest results.

[MarcelRobitaille]

I added scrolling, including keeping the focused element centered when using the arrow keys. Also made some visual improvements.

Peek.2022-07-25.12-05.mp4

[MarcelRobitaille]

I used a mixin to implement this in EditInputField as well and did some other cleanup.

Peek.2022-07-25.15-31.mp4

I should also mention that I fixed the popup overflowing to the right (it will be right-aligned and not aligned with the # if there is not enough space) and I update the popup position when the text wraps.

Peek.2022-07-25.15-52.mp4

[christianlupus]

I just wanted to test the extension and I am facing issues. When I want to select a recipe from the popup (both using enter as well as a mouse click), the popup is not closed and the console shows an error:

---

Another thing that I have seen and that might be a bit suboptimal from UX perspective: When you want to enter a link and type #ap (for apple pie), you can "loose" the popover by pressing ESC or moving the focus to another location (all right). When you decide that you want to continue typing the recipe name, nothing is happening anymore. You have to remove the # in order to trigger the popup again.

Was it possible to extend the functionality such that if you type at the end of any word that started with a single # to open the popover dialog as well?

[seyfeb]

Was it possible to extend the functionality such that if you type at the end of any word that started with a single # to open the popover dialog as well?

One word of caution: this should not lead to repeatedly opening the popup after the user has canceled the selection (with ESC, the arrow keys, or sth.) and continues typing otherwise this would be annoying :D It might make sense if the user has navigated to a completely unrelated part and returns back to the end of the string starting with a #

[christianlupus]

That is true. However, as far as I understand the idea, the user "just" does not have to press enter to be allowed to type anything he/she likes. Only pressing enter or selecting with the pointer/mouse will replace the word with the link. Pressing Esc Enter would allow inserting a line break directly after a word beginning with #.

[MarcelRobitaille]

Sorry about that. I did some major refactoring and didn't test everything well enough. I'll fix it.

[MarcelRobitaille]

I fixed the bug.

I also implemented recovery of the popup when the focus comes back. This will not recover if the popup was dismissed with escape or arrow keys. Is this the desired behaviour? It's hard to see my clicks in the video. I click outside the textarea then I click back inside the textarea.

Peek.2022-07-26.17-45.mp4

[christianlupus]

OK, I tested once more.

The bug I reported is still present: You cannot insert any link to the description field. The other fields do work as these are EditInputGroups. But the description is an EditInputField which is missing the handleSuggestionSelected method used in the mixin.

Second, I think the fix for the hidden popup is not working on my machine:

903-no-links.mp4

[MarcelRobitaille]

Hi @christianlupus. Thanks for testing it. I think the first issue has to do with the markdown editor. I am getting an error related to pasteString: TypeError: field.editor is undefined. I logged this.$refs.inputField, which is a dom node with no editor attribute. I'm not sure how this is supposed to work, and I'm not sure how I could have changed this with this PR.

The video is interesting. I will look into it.

[christianlupus]

Hi @christianlupus. Thanks for testing it. I think the first issue has to do with the markdown editor. I am getting an error related to pasteString: TypeError: field.editor is undefined. I logged this.$refs.inputField, which is a dom node with no editor attribute. I'm not sure how this is supposed to work, and I'm not sure how I could have changed this with this PR.

This is a leftover from an old implementation I removed for security reasons (high-risk npm audit warning). The old editor was a Vue component that declared itself as Markdown editor. It looked much like a plain textarea. As the component has not received any updates for over 2 years, I removed it as stale.

The video is interesting. I will look into it.

FYI: This is made on Linux with Firefox.

[MarcelRobitaille]

I realized that just doing paste on submit was the wrong approach. We need to remove the search text and insert the reference. This must keep the text before and after intact. @christianlupus noticed this in the video where the search text was remaining.

As a result, the issue of the discrepancy between EditInputGroup and EditInputField selection is fixed. I implemented the above logic in a way that it works for both and does not rely on pasteString.

This video shows that the search text is correctly replaced. In previous versions, the search text would remain after the inserted reference.

Peek.2022-07-28.02-12.mp4

I still do not know why the popup is not recovered properly in your video. Would you try the latest version? I added some logging that might help track down what's going on. I also realized it would be possible to implement that feature with css and a sibling selector. That could be a good option as well. It's fewer event listeners to set up every time the component is used at any rate.

[christianlupus]

Now, it seems to work rock-solid. I like it! 💯 🚀

Do you want to clean up the debug logs or comment them out in some other way? Please let me know, when you are finished with the PR.

[MarcelRobitaille]

Glad you like it!

Actually I just noticed one other problem. You can confuse the system if you click inside the textarea to change the caret position.

Peek.2022-07-29.15-29.mp4

GitHub just closes the popup if the caret changes by click, even if the new position is between the # and the next space.

[MarcelRobitaille]

I can remove the logs if you want, but I only added them to track down your recovery bug. I did not change anything about that besides adding the logs, so I expected the bug to still be there.

Also, is there a good way to avoid all of this https://github.com/MarcelRobitaille/cookbook/blob/903-link-suggestions/src/components/EditInputField.vue#L12-L15? Right now, it's duplicated in 4 different places. Plus, you have to remember to call handleSuggestionsPopupKeyDown in the keyUp handler: https://github.com/MarcelRobitaille/cookbook/blob/903-link-suggestions/src/components/EditInputGroup.vue#L289

[christianlupus]

I can remove the logs if you want, but I only added them to track down your recovery bug. I did not change anything about that besides adding the logs, so I expected the bug to still be there.

I have not been able to reproduce the problem. To prevent filling up the console of the production, I added a config flag to hide them by default. Only if the environment variable VERBOSE is set to 1 during running npm run build-dev or similar commands, the debug logging is put out.

This will allow us to debug if the problem comes up again.

Also, is there a good way to avoid all of this https://github.com/MarcelRobitaille/cookbook/blob/903-link-suggestions/src/components/EditInputField.vue#L12-L15? Right now, it's duplicated in 4 different places.

I am not aware of a way to shorten that. You will have to register with all components the appropriate event listeners. Are you only concerned that you want to write less code or because you fear breaking changes in the future if the code grows further?

Plus, you have to remember to call handleSuggestionsPopupKeyDown in the keyUp handler: https://github.com/MarcelRobitaille/cookbook/blob/903-link-suggestions/src/components/EditInputGroup.vue#L289

Here am not sure if this can be simplified. I am thinking if this could work by adding another layer of components as an abstraction that will handle the majority of the event handling. Then the effect would be restricted to a hand full of files. But I have no overview of the current state of the components and I thus cannot provide well-educated feedback.

[christianlupus]

LGTM

[christianlupus]

I just approved. So, you can merge yourself. If you see fit, you can merge now and think about any code style enhancements later or try to enhance directly in this PR. This is up to you, @MarcelRobitaille.