Pressing Ctrl+F twice on topic search should close the Discourse search window

I sometimes want to use the browser search window within topics since the Discourse search function jumps only to posts and not the exact position of the search term. Pressing Ctrl+F works quite well.

However, the Discourse search window doesn’t close automatically when I use the browser’s search function. The first few search results therefore are often within the search window. That’s quite annoying.

Expected behaviour: When I press Ctrl+F the second time, the Discourse search window should close automatically.

2 Likes

Hmm… That may be easier said than done. As by default the keyboard keypress framework that Discourse uses, doesn’t capture keypresses that are made in input boxes (which is why pressing it twice works).

So there is nothing capturing the second pressing of it. And if we did, you’d have us capturing it instead of the browser responding to it… So its a bit of catch-22 here.

After a quick look at the documentation of Mousetrap it looks like the following could work:

  • Add class="mousetrap" to the input field.
  • Close the search window when Ctrl+F is pressed
  • Do not stop the default behaviour (return true instead of false in the event handler)

I’ve starred this, so I’ll take a look when I get around to it. If it works and doesn’t have any side effects, I’ll submit the PR for it.

6 Likes

I want to add that I am very happy you have adopted the keyboard bits! Be the change you want to see, and you are doing exactly that. Kudos.

4 Likes

I submitted a PR for it if anyone would like to take a look and review :smiley_cat:

https://github.com/discourse/discourse/pull/3546

2 Likes

Thanks for your PR. Unfortunately it doesn’t work the way I imagined, but I guess that’s because I didn’t exactly describe the behavior I wanted. I’ll clarify it:

Expected behavior: When I press Ctrl+F the second time, the Discourse search window should close automatically and the browser’s search window should be visible and have focus.

Current behavior without the PR: When I press Ctrl+F the second time, the Discourse search window stays open and the browser’s search window is visible and has focus.

Behavior with the PR: When I press Ctrl+F the second time, the Discourse search window closes, but there is now browser search window.

2 Likes

@gerhard thanks for clarifying :smile: I do find the proposed behavior to be confusing for users as I’m not sure how we can clearly bring across the behavior to users that pressing Ctrl + f twice will first bring up Discourse search and then the browser’s search.

1 Like

But that is exactly what is happening if you press Ctrl+F in a topic with lots of posts. Try it out yourself by pressing it inside a topic with more than 20 posts.

And it is good that this is happening since I want to be able to search using my browser. It has a better search experience. My problem right now is that the Discourse search window stays visible and I get lots of search results from the browser that are within Discourse’s search results. See my screenshot in the first post.

I do understand what you mean and I believe the browser’s search appearing is a bug since Discourse has made the decision to hijack the search on the first ctrl+f.

No, it’s not a bug. That is intentional. You can read about it in older discussions:

2 Likes

I prefer to call it a happy little accident.

1 Like

I submitted a PR that implements the expected behavior:

https://github.com/discourse/discourse/pull/3559

6 Likes

@gerhard I just tested your PR out :smile: Looks good to me and definitely a better experience.

3 Likes

This has regressed on meta :frowning: :cat:

2 Likes

Well, it worked for nearly three weeks since my last fix.
Let’s see how long this one will last. :wink:

https://github.com/discourse/discourse/pull/3903

6 Likes

Should we start an office pool? :laughing:

1 Like

Perhaps a test just to make sure on a long topic, Ctrl + f twice will at least close the search header.

I wasn’t sure how to send a Ctrl + f in the tests but @eviltrout solved it yesterday :smile:

https://github.com/discourse/discourse/blob/84a50a1260c228ad71a1d3dcbe17b50b36507663/test/javascripts/acceptance/composer-test.js.es6#L47-L53

4 Likes