creativecommons
change openverse to bold text and made it default search engine
Fixes
Fixes #108 by @TimidRobot and #97 by @JennySimen
Description
PR selects openverse by default and bold its text
Technical details
Tests
Screenshots
Checklist
- [x] My pull request has a descriptive title (not a vague title like
Update index.md). - [x] My pull request targets the default branch of the repository (
mainormaster). - [x] My commit messages follow best practices.
- [x] My code follows the established code style of the repository.
- [x] I added or updated tests for the changes I made (if applicable).
- [x] I added or updated documentation (if applicable).
- [x] I tried running the project locally and verified that there are no visible errors.
Developer Certificate of Origin
For the purposes of this DCO, "license" is equivalent to "license or public domain dedication," and "open source license" is equivalent to "open content license or public domain dedication."
Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
@ImaJin14 The enter key behavior still seems to remain unchanged after testing via Docker with a fresh cache.
@ImaJin14 The
enterkey behavior still seems to remain unchanged after testing via Docker with a fresh cache.
I'm currently working on it... the click function always defaults to the first button and even after changing that it doesn't reflect changes with a cleared cached... any insights on how I could better address this functionality problem??
@ImaJin14 You might have success targeting a key behavior rather than click, like keydown.
- https://api.jquery.com/keydown/
@ImaJin14 That seems to do the trick, but I did notice an irregularity when testing it. When you hit the enter key, it opens in a tab to the correct url (openverse), but in the original window where the CC Search Portal is open, the selected engine that gets highlighted is ccMixter?
That might be worth addressing since it appears there's still some js code somewhere setting ccMixter as the "highlighted" engine in the UX.
@ImaJin14 That seems to do the trick, but I did notice an irregularity when testing it. When you hit the enter key, it opens in a tab to the correct url (openverse), but in the original window where the CC Search Portal is open, the selected engine that gets highlighted is ccMixter?
That might be worth addressing since it appears there's still some js code somewhere setting ccMixter as the "highlighted" engine in the UX.
I've noticed that too... I'll address it as soon as I'm done prunning my docker system and rebuilding it... I've noticed my files aren't rendering I'm real time even after clearing my cache...
@possumbilities please for your review and insight if any further changes need to be done....
@ImaJin14 I've tested it several times in Docker with all manner of cache clearing and it still seems to be shifting to ccMixter in the original window, after launching the tab. Your sure it's working as expected?
@ImaJin14 I've tested it several times in Docker with all manner of cache clearing and it still seems to be shifting to ccMixter in the original window, after launching the tab. Your sure it's working as expected?
I removed the function that always defaulted it to the first engine button... Please for your insights on how to go about improving the ux problem....
I found this issue quite intriguing and decided to take a look at it to see if I can throw in some ideas that can help solve the issue 😁.
I did some investigation and here are some findings:
-
To make openverse default, there are already some code to facilitate that while removing some lines of code (clean up) Setting the default engine to
openversevar default_engine = "openverse";Change the setting of engine to default_engine
if (engine == null || !engine || engine == "") { engine = default_engine; } ``` So, in setEngine(e) you can remove the second if statementif (typeof e == "string") { engine = e; } else { if (e) { engine = e.value; } }
-
To fix the ccMixter bug on keypress (I noticed this bug is also on the live site so it might not be related to this ticket) Seems like the bug happens when ever the ENTER key is pressed while the search is focused. So if I am on the default engine (or any chosen engine) and I press the ENTER btn, ccMixter is selected and the search results are for ccMixter.
This means when the key is pressed, the first engine is always selected.
A solution could be to prevent the default behaviour of the form submission when key is pressed and then get the current selected engine and continue with the flow. Something like this
$("#search_form").keypress(function(event) { if (event.which == 13) { event.preventDefault(); setEngine($("button[value=" + engine + "]").first().attr("id")); doSearch(); }});
This might not be the best approach but what do you think 🤔? @possumbilities @ImaJin14
I found this issue quite intriguing and decided to take a look at it to see if I can throw in some ideas that can help solve the issue 😁.
I did some investigation and here are some findings:
To make openverse default, there are already some code to facilitate that while removing some lines of code (clean up) Setting the default engine to
openverse
var default_engine = "openverse";Change the setting of engine to default_engine
if (engine == null || !engine || engine == "") { ngine = default_engine;} ``` So, in setEngine(e) you can remove the second if statement
if (typeof e == "string") { engine = e; } else { if (e) { engine = e.value; } } ``` 2. To fix the ccMixter bug on keypress (I noticed this bug is also on the live site so it might not be related to this ticket) Seems like the bug happens when ever the ENTER key is pressed while the search is focused. So if I am on the default engine (or any chosen engine) and I press the ENTER btn, ccMixter is selected and the search results are for ccMixter. This means when the key is pressed, the first engine is always selected. A solution could be to prevent the default behaviour of the form submission when key is pressed and then get the current selected engine and continue with the flow. Something like this ``` $("#search_form").keypress(function(event) { if (event.which == 13) { event.preventDefault(); setEngine($("button[value=" + engine + "]").first().attr("id")); doSearch(); }}); ``` This might not be the best approach but what do you think 🤔? @possumbilities @ImaJin14
The ccmixter bug on key press doesn't go away when the default event is prevented... Also there are a few codebase that can't be removed from the setEngine code because it helps in engine selection....
The only bug left on this PR is the ccmixter bug on enter key
I found this issue quite intriguing and decided to take a look at it to see if I can throw in some ideas that can help solve the issue 😁. I did some investigation and here are some findings:
- To make openverse default, there are already some code to facilitate that while removing some lines of code (clean up) Setting the default engine to
openversevar default_engine = "openverse";Change the setting of engine to default_engineif (engine == null || !engine || engine == "") { engine = default_engine;}
So, in setEngine(e) you can remove the second if statementif (typeof e == "string") { engine = e; } else { if (e) { engine = e.value; } } ```
To fix the ccMixter bug on keypress (I noticed this bug is also on the live site so it might not be related to this ticket) Seems like the bug happens when ever the ENTER key is pressed while the search is focused. So if I am on the default engine (or any chosen engine) and I press the ENTER btn, ccMixter is selected and the search results are for ccMixter.
This means when the key is pressed, the first engine is always selected.
A solution could be to prevent the default behaviour of the form submission when key is pressed and then get the current selected engine and continue with the flow. Something like this
$("#search_form").keypress(function(event) { if (event.which == 13) { event.preventDefault(); setEngine($("button[value=" + engine + "]").first().attr("id")); doSearch(); }});This might not be the best approach but what do you think 🤔? @possumbilities @ImaJin14
The ccmixter bug on key press doesn't go away when the default event is prevented... Also there are a few codebase that can't be removed from the setEngine code because it helps in engine selection....
The only bug left on this PR is the ccmixter bug on enter key
You are not actually removing code in use, it is more of refactoring. If you set a default engine, then there is no need to check if if (e == "_random") or if (e == "openverse") since engine = e; will pass all usecases for the condition if (typeof e == "string").
There would always be a search engine and no special usecase, hence removing the codition to test _random or openverse.
Also, preventing the default behaviour is not the only step, if you look at the sample code I added, we are handling the key press the exact same way we handle the button onclick. Using event.preventDefault(); just stops it from moving to ccMixter then you can continue to get the selected engine and do the search. I tested the code and it seems to work fine.
You can test the sample code (which you can still refactor) and I think it should work or better still ask in the slack channel for more suggestions. What do you think @possumbilities @ImaJin14 ?
@ImaJin14 @JennySimen
To fix the ccMixter bug on keypress (I noticed this bug is also on the live site so it might not be related to this ticket)
If this is truly the case, my thoughts lean toward we need to document a new Issue, and that this PR can likely go ahead without resolving it. And a separate PR can fix this bug.
@ImaJin14 @JennySimen
To fix the ccMixter bug on keypress (I noticed this bug is also on the live site so it might not be related to this ticket)
If this is truly the case, my thoughts lean toward we need to document a new Issue, and that this PR can likely go ahead without resolving it. And a separate PR can fix this bug.
It's a problem that occurs right on the live site... I thought it would have been nice to resolve it within this PR but since another issue could be opened for the bug I will just bring this PR up to speed with branch main....
@possumbilities branch is currently up to speed with branch main and issue #171 aims to resolve the ccmixter UX bug
