-
-
Notifications
You must be signed in to change notification settings - Fork 7
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 functiontionality is disabled (text box not selectable) #8
Comments
Hi @anenadic, Thank you for bringing this up! I've transferred this to the workbench repository since this particular issue is for the workbench as a whole as opposed to the documentation. Short answerThe search functionality is currently disabled and I apologise for not flagging it as disabled. I am working on implementing it in an accessible manner, but it may take some time. Long answerThe search bar exists because we asked for it during the design of the frontend. The designer added the search bar with a button and the frontend contractors placed it in the lesson, but without functionality behind it because how exactly search results would be displayed was unclear. That being said, our infrastructure is built on top of {pkgdown} and search functionality does exist in {pkgdown}, as you can see from the {sandpaper} package website, but we do not import any of their JavaScript or CSS the structure of the website is so different. I thought I opened an issue to add functionality to the search button, but it turns out that I only generated a commit that begins the process with a note about what I found:
The steps for me to resolve this are:
|
Note about the search functionality, I think the appropriate path is to take the approach of Mozilla Developer N(etwork)? and implement the search field that does two things:
The search page they have looks like this: https://developer.mozilla.org/en-US/search?q=localStorage |
@zkamvar I just ran into this as well. Do you plan to work on this soon? If not, it might be better to hide the search box in the meantime to reduce confusion. |
Is there currently another smart way of searching within a whole lesson? This is useful for maintainers. Cheers |
At the moment, a good way to search the whole lesson is to visit the all in one page, which is located at the bottom of the schedule: https://carpentries.github.io/workbench/transition-guide.html#aio-instructor The page is always called "aio.html", so you can visit there directly by adding |
And I apologise on my delay with this issue. There are many things that have come up since it was opened -_- |
No worries, I'm aware you have a lot of things to think about. The one-page version works perfectly, thank you. |
The search box is not selectable. I've tried on multiple SWC lessons, using two different browsers. |
@kekoziar, it has not yet been enabled. See #8 (comment) for details. I have gotten part of the way with carpentries/varnish#86, which you can see the result in my fork of instructor training (https://zkamvar.github.io/instructor-training/), but it's not quite ready as there is still a bit of accessibility testing that needs to happen. |
Got it. Thanks. I saw the initial "The search box at the top only provides for search on the actual page and not search across the whole lesson site," and saw that it wasn't enabled, but missed that the box is not selectable. I thought it was only searching partially. Do you think you can edit your initial answer to say "The search functionality is currently disabled (the search box is not selectable)"? |
Given this is a long outstanding issue I wonder if it is possible/worth while to make it much clearer that search doesn't work, or even remove the search bar completely from the display? It isn't at all obvious to me that the search is disabled - I tried clicking many times, checked across browsers, before realising that the field was disabled by checking the code. I think it would be a better solution to remove the search box until it works. But if this is not possible perhaps it needs to be more obvious it is not implemented yet - by some onscreen text maybe? |
Hi @ostephens - the transition to the Workbench has been a lengthy process and this has been on the wayside for a while, but @zkamvar has been making progress in the linked draft PR: carpentries/varnish#86. We'd like to try and get this done by the end of the year, but as we have a lot of other higher priority work, this might not happen. We might be able to fit in a hotfix to more clearly render the search box as disabled, perhaps with a "Disabled" placeholder or some such, but again, I'd have to catch up with Zhian to see if this makes sense given his own thoughts about progress with the draft PR. |
How about |
@froggleston and I have made some updates to Zhian's PR for introducing search functionality and are have reached out to Maintainers to test. The Instructions for Testing Search Bar in Workbench document includes instructions for testing locally and on GitHub. We are asking folks to provide feedback either to this issue or in response to this email on the Maintainers TopicBox by 13 March. |
Thanks for tackling this important feature. Here are some of my impressions.
Edited by @ErinBecker to add numbers for easy reference in discussion below. |
I've just had a go at testing this locally and on this fork.
|
Thank you @quist00 and @astroDimitrios for going through the testing process and providing detailed feedback. This is really helpful. I have some inkling of how to go about solving issues 1, 2, and 3 raised by @astroDimitrios and possibly issue 5 raised by @quist00. Issues 1-4 and 6-7 raised by @quist00 are pretty far outside of my comfort zone and into some detailed CSS manipulation. @froggleston - I'm going to see what I can do about addressing @astroDimitrios's 1,2,3 and @quist00's 5 and will post updates here. |
Just tested locally (on the instructor training). In addition to what @astroDimitrios brought up:
I did find one other glitch: when results are in the instructor notes page (at least on the instructor training), I get |
Thank you @ndporter @astroDimitrios and @quist00 for taking the time to test out this feature and provide detailed feedback. It's really great to see members of the community engaging with the Workbench and helping to make it better! After banging my head against these issues for a while, it seems to me that using the Algolia Docsearch package to implement search on our lessons (the strategy used in carpentries/varnish#86 and carpentries/sandpaper#495 and to which the feedback above is directed), isn't a good out-of-the-box solution for us, as it doesn't give us the desired look and feel. Our ability to customize Docsearch is also very limited. Pkgdown does also provide a built-in search capability, but according to the docs, "the only available customisation is excluding some paths from the search index." Another option we've considered is embedding a Google search widget, but the time it takes Google to re-index a page depends on a lot of factors, and it may be weeks before changes to the lesson show up in searches. Given these limitations, we're going to move forward with @kekoziar's suggestion of making the option to search the all in one page more visible. My next update here will include a link to a set of PRs (in progress) for implementing this change. varnish PR: carpentries/varnish#131 |
Thanks to @ErinBecker 's awesome efforts, the problematic search box has been replaced by a button that will take you to the All in One page that will facilitate easier searching by the browser's built-in search feature. There were simply too many issues with the Algolia solution that would have taken much longer to fix, so we felt this was the next best thing. Thanks @ErinBecker ! |
The search box at the top only provides for search on the actual page and not search across the whole lesson site. Is this the intended behaviour?
The text was updated successfully, but these errors were encountered: