-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: Operational OEIS search bar #410
Conversation
This PR attempts to resolve numberscope#407. I added a half-dozen OEIS sequences as rapidly as I could and didn't get any errors. So hopefully this has squashed the problem, but it's hard to be certain.
In order to be sure that parameter values have their "final" values in the case of an update (e.g., by calling the Paramable validate() method), it is necessary for a cascade of functions to be made async. This commit makes those changes and then awaits the validate() in the user interface, so that the URL won't be updated until all of the consequences of validate() have occurred (which may, for example, involve changing the parameter values, perhaps even the one that originally had its tentative value modified, necessitating the validate() call). Resolves numberscope#403.
First main feedback: The mouse is a "cursor" mouse over the part you need to click on to load the sequence. It turns into a pointer finger only on the link to the OEIS url. So I was confused trying to figure out where I can load the sequence. Is this related to the fact that I get a |
Another small observation: There's no way to "back out" of a search except for deleting everything in the search box.... I kind of expected clicking somewhere else or something to back out. I'm not sure if this is what you had in mind or not, and I'm not sure what is the best behaviour, but I thought I would mention that it caused me a little bit of "exploration click time" that didn't result in anything. |
No, it's just that I need to add the CSS command to change the cursor. Will do as soon as I have the opportunity. |
Happy to implement other "back out" gesture(s) -- but just let me know what would be the intuitive action(s) to back out. |
Yes, that was literally one line, which is now committed. By the way, I meant to ask: should we bolden or highlight in some other way occurrences of the search string in the displayed names of the search results? |
I came back to it and played with it again today and I found myself just trying to click anywhere outside the search box/results area. I think that would be a perfectly fine solution. Then one thing to make sure is that clicking back in the search box, even without further typing, should make the results open again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to implement this but found I had a question: Right now, when the search results box is popped up, clicking on one of the still-visible sequence preview cards closes the search results and the Change Sequences modal and switches to the clicked sequence (even if that card is only partly visible, but the click is outside the search results. Should that behavior persist, OR while the search results is up, do you want a click outside the search results to close the search results and not do anything else? Thanks for letting me know. |
My reaction is that the latter would be less confusing. But you can go with what seems to make sense to you. |
OK, I have committed code intended to result in this latter behavior. You can pull and try it out at your leisure. |
- Return to the OEIS ID being a link but add the Wikipedia "external link" icon - Highlight the description of each sequence as you mouse over it, to provide clue you can click on it (to add as a sequence). The color used is a lightened version of the "primary" Numberscope color. - Display a message when there are no sequences to show as a result of a search. Note this can occur because there are _too many_ matches, or _no_ matches, and there is no way to tell which is the case from the OEIS api response, unfortunately. The message reflects this. - Fixed bug that results were being cached under the wrong search term if the searcher typed more while the search was executing. - Make sure that enter/clicking on magnifier always fires search
All right, I think that I've done everything open on this PR. From the latest commit message:
Of course if you see anything else that needs changing/fixing or if there is anything in the code that needs to be improved, I am happy to develop this PR further. Thanks! |
P.S. In particular I don't love the phrasing I came up with for an "unsuccessful" search, namely ...... search for foobar gave no/too many results So if anyone has better/clearer wording for this that is still relatively short, I would be happy to change. |
Two comments: (1) clicking on magnifier still gives me a tooltip and stops search (2) for the message, you could try '....... search for foobar failed (OEIS reports no results or too many results)' One advantage of this phrasing is it attempts to pass the blame for the ambiguity ;) But simpler may be '....... search for foobar failed (no results or too many results)' Everything else works great! |
OK, I think I've taken care of both of those things. |
When the search doesn't come back (axios/network/cors error) currently there's a popup and all interaction is interrupted for the user... but I think in the context of search especially, we should just continue (the same as if the user clicks "ok" on the popup) because it generally resolves itself at that point. At least this is what's happening for me right now. This might be appropriate to this PR or it might be something for post delft todos discussion? Ideas? |
I don't think you pushed the commits. |
Oops, embarrassing. I have now. |
Ok, it looks good now. The tooltip pops up mouseover for the magnifier, which is good. Clicking causing search is essentially unverifiable at the moment for me, since my search is returning fast enough on typing (meaning, by the time I move my mouse and click, the search already fired). But that's ok, I don't see any suggested change. I think the only outstanding issue is the popups on network errors (comment earlier today), which maybe should be disabled during search? Or maybe even entirely. Those popups haven't served a purpose for me other than interrupting things that later fix themselves. |
OK, I will create and push a commit that catches network errors in the OEIS search and adds a message that the search was temporarily unavailable, please try again, and will not cache that outcome. |
Oh perfect, that sounds like a good solution. |
OK, I have added such a commit. I will be honest though: I spent a while mashing the keyboard and could not invoke the new error-handling code. Maybe the time of day? (Less load on OEIS server?) So let me know how it works for you when you try it. |
Ok, I just tried it and was able to provoke a server error in the console that was not popped up and not apparent in the browser/user experience. So that's perfect. I don't know why I'm better at making the server mad. :) |
Ok, I believe that everything is in order now with this, shall I squash and merge? |
You're the reviewer ;-) anyhow, fine by me if you squash and merge -- I don't know of anything else outstanding. |
* refactor: move the OEIS search bar into its own component. * doc: Basic Modfill -> two OEIS-based Specimens in Gallery * feat: Dummy search results that pop up on input * feat: Call a dummy search cmd on input * feat: list of links and labels for search results * feat: Clicking on a result adds it * fix: When you click on link in results don't also add * feat: Actually search the OEIS. * feat: Cache search results and reopen search on click. * ui: Layout and caption changes per design discussions * fix: Never cache OEIS sequences with infinite indices This PR attempts to resolve numberscope#407. I added a half-dozen OEIS sequences as rapidly as I could and didn't get any errors. So hopefully this has squashed the problem, but it's hard to be certain. * fix: Don't update URL until after parameter values have settled In order to be sure that parameter values have their "final" values in the case of an update (e.g., by calling the Paramable validate() method), it is necessary for a cascade of functions to be made async. This commit makes those changes and then awaits the validate() in the user interface, so that the URL won't be updated until all of the consequences of validate() have occurred (which may, for example, involve changing the parameter values, perhaps even the one that originally had its tentative value modified, necessitating the validate() call). Resolves numberscope#403. * ui: show finger cursor over search results * make oeis.org link open new tab * info icon for info in search results * ui: click off list of OEIS search results closes it * fix: Restore sequences adding when you click on them in result list. * fix: Various fixes to OEIS search bar/results per meeting - Return to the OEIS ID being a link but add the Wikipedia "external link" icon - Highlight the description of each sequence as you mouse over it, to provide clue you can click on it (to add as a sequence). The color used is a lightened version of the "primary" Numberscope color. - Display a message when there are no sequences to show as a result of a search. Note this can occur because there are _too many_ matches, or _no_ matches, and there is no way to tell which is the case from the OEIS api response, unfortunately. The message reflects this. - Fixed bug that results were being cached under the wrong search term if the searcher typed more while the search was executing. - Make sure that enter/clicking on magnifier always fires search * fix: Don't make switcher cards for dummy message IDs * fix: Really make sure magnifier puts/keeps search results up; reword message * fix: catch errors in OEIS searching and report a 'soft' failure --------- Co-authored-by: Kate Stange <[email protected]>
* refactor: move the OEIS search bar into its own component. * doc: Basic Modfill -> two OEIS-based Specimens in Gallery * feat: Dummy search results that pop up on input * feat: Call a dummy search cmd on input * feat: list of links and labels for search results * feat: Clicking on a result adds it * fix: When you click on link in results don't also add * feat: Actually search the OEIS. * feat: Cache search results and reopen search on click. * ui: Layout and caption changes per design discussions * fix: Never cache OEIS sequences with infinite indices This PR attempts to resolve numberscope#407. I added a half-dozen OEIS sequences as rapidly as I could and didn't get any errors. So hopefully this has squashed the problem, but it's hard to be certain. * fix: Don't update URL until after parameter values have settled In order to be sure that parameter values have their "final" values in the case of an update (e.g., by calling the Paramable validate() method), it is necessary for a cascade of functions to be made async. This commit makes those changes and then awaits the validate() in the user interface, so that the URL won't be updated until all of the consequences of validate() have occurred (which may, for example, involve changing the parameter values, perhaps even the one that originally had its tentative value modified, necessitating the validate() call). Resolves numberscope#403. * ui: show finger cursor over search results * make oeis.org link open new tab * info icon for info in search results * ui: click off list of OEIS search results closes it * fix: Restore sequences adding when you click on them in result list. * fix: Various fixes to OEIS search bar/results per meeting - Return to the OEIS ID being a link but add the Wikipedia "external link" icon - Highlight the description of each sequence as you mouse over it, to provide clue you can click on it (to add as a sequence). The color used is a lightened version of the "primary" Numberscope color. - Display a message when there are no sequences to show as a result of a search. Note this can occur because there are _too many_ matches, or _no_ matches, and there is no way to tell which is the case from the OEIS api response, unfortunately. The message reflects this. - Fixed bug that results were being cached under the wrong search term if the searcher typed more while the search was executing. - Make sure that enter/clicking on magnifier always fires search * fix: Don't make switcher cards for dummy message IDs * fix: Really make sure magnifier puts/keeps search results up; reword message * fix: catch errors in OEIS searching and report a 'soft' failure --------- Co-authored-by: Kate Stange <[email protected]>
* refactor: move the OEIS search bar into its own component. * doc: Basic Modfill -> two OEIS-based Specimens in Gallery * feat: Dummy search results that pop up on input * feat: Call a dummy search cmd on input * feat: list of links and labels for search results * feat: Clicking on a result adds it * fix: When you click on link in results don't also add * feat: Actually search the OEIS. * feat: Cache search results and reopen search on click. * ui: Layout and caption changes per design discussions * fix: Never cache OEIS sequences with infinite indices This PR attempts to resolve numberscope#407. I added a half-dozen OEIS sequences as rapidly as I could and didn't get any errors. So hopefully this has squashed the problem, but it's hard to be certain. * fix: Don't update URL until after parameter values have settled In order to be sure that parameter values have their "final" values in the case of an update (e.g., by calling the Paramable validate() method), it is necessary for a cascade of functions to be made async. This commit makes those changes and then awaits the validate() in the user interface, so that the URL won't be updated until all of the consequences of validate() have occurred (which may, for example, involve changing the parameter values, perhaps even the one that originally had its tentative value modified, necessitating the validate() call). Resolves numberscope#403. * ui: show finger cursor over search results * make oeis.org link open new tab * info icon for info in search results * ui: click off list of OEIS search results closes it * fix: Restore sequences adding when you click on them in result list. * fix: Various fixes to OEIS search bar/results per meeting - Return to the OEIS ID being a link but add the Wikipedia "external link" icon - Highlight the description of each sequence as you mouse over it, to provide clue you can click on it (to add as a sequence). The color used is a lightened version of the "primary" Numberscope color. - Display a message when there are no sequences to show as a result of a search. Note this can occur because there are _too many_ matches, or _no_ matches, and there is no way to tell which is the case from the OEIS api response, unfortunately. The message reflects this. - Fixed bug that results were being cached under the wrong search term if the searcher typed more while the search was executing. - Make sure that enter/clicking on magnifier always fires search * fix: Don't make switcher cards for dummy message IDs * fix: Really make sure magnifier puts/keeps search results up; reword message * fix: catch errors in OEIS searching and report a 'soft' failure --------- Co-authored-by: Kate Stange <[email protected]>
* refactor: move the OEIS search bar into its own component. * doc: Basic Modfill -> two OEIS-based Specimens in Gallery * feat: Dummy search results that pop up on input * feat: Call a dummy search cmd on input * feat: list of links and labels for search results * feat: Clicking on a result adds it * fix: When you click on link in results don't also add * feat: Actually search the OEIS. * feat: Cache search results and reopen search on click. * ui: Layout and caption changes per design discussions * fix: Never cache OEIS sequences with infinite indices This PR attempts to resolve #407. I added a half-dozen OEIS sequences as rapidly as I could and didn't get any errors. So hopefully this has squashed the problem, but it's hard to be certain. * fix: Don't update URL until after parameter values have settled In order to be sure that parameter values have their "final" values in the case of an update (e.g., by calling the Paramable validate() method), it is necessary for a cascade of functions to be made async. This commit makes those changes and then awaits the validate() in the user interface, so that the URL won't be updated until all of the consequences of validate() have occurred (which may, for example, involve changing the parameter values, perhaps even the one that originally had its tentative value modified, necessitating the validate() call). Resolves #403. * ui: show finger cursor over search results * make oeis.org link open new tab * info icon for info in search results * ui: click off list of OEIS search results closes it * fix: Restore sequences adding when you click on them in result list. * fix: Various fixes to OEIS search bar/results per meeting - Return to the OEIS ID being a link but add the Wikipedia "external link" icon - Highlight the description of each sequence as you mouse over it, to provide clue you can click on it (to add as a sequence). The color used is a lightened version of the "primary" Numberscope color. - Display a message when there are no sequences to show as a result of a search. Note this can occur because there are _too many_ matches, or _no_ matches, and there is no way to tell which is the case from the OEIS api response, unfortunately. The message reflects this. - Fixed bug that results were being cached under the wrong search term if the searcher typed more while the search was executing. - Make sure that enter/clicking on magnifier always fires search * fix: Don't make switcher cards for dummy message IDs * fix: Really make sure magnifier puts/keeps search results up; reword message * fix: catch errors in OEIS searching and report a 'soft' failure --------- Co-authored-by: Kate Stange <[email protected]>
By submitting this PR, I am indicating to the Numberscope maintainers that I have read and understood the contributing guidelines and that this PR follows those guidelines to the best of my knowledge. I have also read the pull request checklist and followed the instructions therein.
This commit makes the OEIS search bar operational, and reconfigures it according to group discussions. I don't think we ever discussed the details of the presentation of the returned results themselves, so I am very much open to feedback and suggestions on the workings of the bar.
This is the final planned PR for the sake of cleaning/merging #360, and as such, supersedes that PR.