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

Search on category select without JavaScript #2740

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

Bnyro
Copy link
Member

@Bnyro Bnyro commented Sep 9, 2023

What does this PR do?

  • The implementation is change to work without javascript: When enabled, the HTML page contains buttons instead of checkboxes.
  • Since JavaScript is not required anymore for it, the setting is moved to the ui preferences and the plugin for it, that used JavaScript, got removed

Why is this change important?

SearXNG have two modes to pick the categories:

  • one as other search engine: when clicked, it sends the user query
  • multiple categories: the user can pick multiple categories and then send the query.

The HTML always use checkboxes and labels --> some Javascript send the query in the first mode when the user clicks on a category.

This creates to UX issues:

  • screen reader and text browser read or show the categories as check boxes.
  • users who disable Javascript can't use the first mode.

This PR intends to fix that.

How to test this PR locally?

  • make run : check in the multiple screen configuration (RTL, dark mode, mobile, tablet, desktop).
  • I've tested it on Firefox Desktop, Chromium Desktop and Firefox on Android alredy

Related issues

closes #1414
closes #2709

(the description has been taken from #2709 by @dalf and extended with additional info)

@dalf
Copy link
Member

dalf commented Sep 9, 2023

After some quick tests:

  • it works nicely (FF, Chrome, w3m)
  • there is a lack of feedback when the user click on category. The browser shows quickly a highlight and then it is like I've never clicked on the category. Of course, the page but a visual hint would help.

@Bnyro
Copy link
Member Author

Bnyro commented Sep 9, 2023

there is a lack of feedback when the user click on category. The browser shows quickly a highlight and then it is like I've never clicked on the category. Of course, the page but a visual hint would help.

I've seen in your PR, that you were highlighting the clicked button with a square border (https://github.com/searxng/searxng/pull/1414/files#diff-4f244eec7a4a314e8d7e0f7d65ea91ba85afdf13035e202b8aaf9f27a1397dfbR79), however I think it's looking better if the new selection is underlined like the current selected page.

I've been trying to not highlight the previously selected category anymore when a new one is chosen, however I couldn't make it work unfortunately since we would need :has() for it, which Firefox does not yet support.

@Bnyro
Copy link
Member Author

Bnyro commented Sep 10, 2023

Should we just do the :has() thing when supported (any browser except Firefox does)? There won't be any negative impact from that on Firefox, the previous elected category just won't be deselected until the new results page loads.

@dalf
Copy link
Member

dalf commented Sep 11, 2023

Should we just do the :has() thing when supported (any browser except Firefox does)?

Is it a mess to use :has() and fallback to javascript for FF?

So the UX is degraded only when JS is disable on FF : in this case, the users will know that page is loading.

@Bnyro
Copy link
Member Author

Bnyro commented Sep 11, 2023

Is it a mess to use :has() and fallback to javascript for FF?

Sounds like a good idea, I'll see if I can make that work tomorrow :)

@dalf
Copy link
Member

dalf commented Sep 11, 2023

I don't know if @supports can be helpful.

@Bnyro
Copy link
Member Author

Bnyro commented Sep 11, 2023

I think it's easier to use document.querySelector("html:has(body)") or something similar to check if it's supported. Not sure if we can use the @support CSS query with JavaScript.

When we use :has() while the browser doesn't support it, it should be ignored automatically, so we just need to use the above test to see whether :has() is unsupported, and if yes, add some onclick listeners to deselect the old selected category.

@Bnyro
Copy link
Member Author

Bnyro commented Sep 12, 2023

We're now doing what you suggested above, seems to be working very well!

I've tested this on Firefox and Chromium (both on Desktop), it would be great if you can test some other browsers you might use if you want to just to make sure.

@return42
Copy link
Member

FYI: I did a rebase on master branch to solve a conflict in the static files.

I did a small functional test / PR seems to work .. @Bnyro @dalf is this PR ready for review?

@Bnyro
Copy link
Member Author

Bnyro commented Sep 14, 2023

I did a small functional test / PR seems to work .. @Bnyro @dalf is this PR ready for review?

Ready to review from my side 👍

Bnyro and others added 2 commits September 18, 2023 20:58
Co-authored-by: Alexandre Flament <alex@al-f.net>
Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

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

Works like a charm :)

FYI: I had to rebase to solve a conflict in the static build files.

@return42 return42 merged commit 09935e2 into searxng:master Sep 18, 2023
8 checks passed
@Bnyro Bnyro deleted the search-on-category-select branch September 18, 2023 19:37
@return42
Copy link
Member

@Bnyro @dalf since we merged this PR the "search on language select" does no longer work .. same with "time-range" and "safe-search" select .. all three drop down boxes do no longer trigger a new search .. do you have a clue what the reason could be / maybe the solution is quite simple, I just don't see it yet ..

@Bnyro
Copy link
Member Author

Bnyro commented Sep 19, 2023

Hmm, that's strange, I'll look into that later.

@dalf
Copy link
Member

dalf commented Sep 19, 2023

... since we merged this PR

There is a tsunami of changes that I can't cope with.
I will try to look at it later.

EDIT: changes = all the PRs these last days.

@Bnyro
Copy link
Member Author

Bnyro commented Sep 19, 2023

https://github.com/searxng/searxng/pull/2740/files#diff-533047a3107c9356db35bd9842440c37ba0e8fa48c1efb3239bce84aaf3a1915R154:

searxng.settings.search_on_category_select is no longer set to true and hence the code in the if bracket isn't running. Not sure why searxng.settings.search_on_category_select is not set to true however.

return42 added a commit to return42/searxng that referenced this pull request Sep 19, 2023
Small addendum to searxng#2740; search_on_category_select is now no longer a plugin.

Related: searxng#2740 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42
Copy link
Member

is no longer set to true and hence the code ..

thanks .. with that hint it was easy to fix --> #2817

There is a tsunami of changes that I can't cope with. .. I will try to look at it later.

If I address @dalf, please do not feel obliged ... no need to answer in a short time ... each as he has time and desire ..

@Bnyro point me in the right direction .. everything is fine 👍

@Bnyro
Copy link
Member Author

Bnyro commented Sep 19, 2023

Awesome 🎉

return42 added a commit that referenced this pull request Sep 19, 2023
Small addendum to #2740; search_on_category_select is now no longer a plugin.

Related: #2740 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42
Copy link
Member

@Bnyro @dalf if you have time for; can you explain with a few words how it works to send a form:

A. with "search on category select" enabled we have one active category
B. with "search on category select" enabled we have n active categories

I have some problems to understand how it works / may first read my comment on:

I don't expect a long description, just a short hint about the trick, which I don't understand at the moment.

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.

Search on category select support without JS
3 participants