DistributedProofreaders / dproofreaders

Distributed Proofreaders is a web application intended to ease the process of converting public domain books into e-texts.
https://www.pgdp.net
GNU General Public License v2.0
46 stars 28 forks source link

Add new "site search" page #1184

Closed cpeel closed 2 months ago

cpeel commented 2 months ago

This adds a new "site search" page. Users can either click on the search icon in the header or use the / hotkey in any non-input field on a site page. This has been available for public preview & discussion in the New site search feature forum.

Sandbox: https://www.pgdp.org/~cpeel/c.branch/site-search/

srjfoo commented 2 months ago

I don't know if this has the potential for being a problem or not, but in testing, I've found myself using the / from the help page, which, of course, has the form input box highlighted and active. Which, of course, breaks whatever I'm searching for. Would it be reasonable to clear the form if a / is typed (including being cleared of the /? Or maybe the / should redirect to the form in the header?

For that matter, once the search term is entered, if it's not a valid search, the cursor moves to the beginning of the search string that's still in the form. Should it be being cleared if it's an invalid search term? I think what I'm getting at is, if the user does get an invalid search response, is there a way to get out of it without using the mouse other than editing the search field, or is that just one of the drawbacks?

Observations:

cpeel commented 2 months ago

For failed searches, my inclination was to put the failed search terms in the box in case someone wanted to edit them and search again. If someone fat-fingers something like searching for titl:mark twain would they rather add the e or just type the whole thing over again? I'm not stuck on this and we could just show an empty input field, but that was my rationale.

We could prevent the search box on the page from having focus. That would make / work on the page again when it loads.

We could do a combination of either of the above.

I don't want to do anything special for / within that search box because that would be extremely unexpected -- when you're in an input field you expect text to be input, not have it not show up or have it move you outside the focus of the field.

The wiki search is just a basic wiki search

Good call. I just made it do a full search which I think will be better.

Prefix searches are exact string, not substring

Correct. This was by design -- the full string or the alias works, but not a subset.


I also meant to add a text to let users know they can use / to bring up the search field on any page. I'll see about getting that in there now.

cpeel commented 2 months ago

I've addressed most of the PR comments (thanks @bpfoley) and added a tip on the use of / at the bottom.

After thinking about the auto-focus and failed search terms in the box, what I've decided is that we should think about 2 primary users:

So I've removed the auto-focus from the page in hopes that it works for both users.

srjfoo commented 2 months ago

For failed searches, my inclination was to put the failed search terms in the box in case someone wanted to edit them and search again. If someone fat-fingers something like searching for titl:mark twain would they rather add the e or just type the whole thing over again? I'm not stuck on this and we could just show an empty input field, but that was my rationale.

Perfectly reasonable thinking.

We could prevent the search box on the page from having focus. That would make / work on the page again when it loads.

I like the idea of removing focus on the search box on a failed search so that the / works. Would that make it harder for those who want to edit the input on the help page?

We could do a combination of either of the above.

How would a combination work?

I don't want to do anything special for / within that search box because that would be extremely unexpected -- when you're in an input field you expect text to be input, not have it not show up or have it move you outside the focus of the field.

I was really pretty much just tossing stuff at the wall in hopes that something might be helpful 😁

The wiki search is just a basic wiki search

Good call. I just made it do a full search which I think will be better.

I really like that -- it should make it easier, for instance, to search for a template. Or a file. (Initially, when I tried the updated version, I did it without the :, and nearly said "butbutbut it's not working! 😆 )

Prefix searches are exact string, not substring

Correct. This was by design -- the full string or the alias works, but not a subset.

That's fine -- I just wanted to make sure it was intentional.

I also meant to add a text to let users know they can use / to bring up the search field on any page. I'll see about getting that in there now.

👍

cpeel commented 2 months ago

1: If you put in something that it doesn't understand (like ab) you get a warning message. Then do a sensible search (like ah). Then go back to the previous page, you get to the page with the warning message. It seems like it would be better to get back to the raw page or have 'ah' filled in.

I agree this is not ideal. To do that we would need to not ever put the failed search term in the search box and at least right now I think that's the better trade-off. It's quite possible that I'm wrong and we should just never put the failed search term in the box and if the consensus (between the devs in this PR or users post-deploy) is that we don't then I'm happy to change that.

2: If you search for a language without capitalizing the first letter, the search understands the language correctly but if you then then do a refine search, the search does not remember the language.

Good find. I don't see an easy way to fix this without doing a fair amount of introspection on the data on the site search page itself. Right now you can search for parts of languages "eng", "english", "engli" and they all work to search, but will all fail if you refine it. I think we should either accept this limitation or remove language as something you can search on with the site search (which is my preference as a project language isn't going to be highly selective). @bpfoley you requested this feature, what do you think?

bpfoley commented 2 months ago

Hmm. As you say, the selectivity of language on its own is pretty low and I'd imagine much of the time language specialists will also work in a specific round, so you really want to be able to search for, e.g. Hawaiian and F1. If refine search isn't working and is a lot of extra work to fix, I'd say it's probably not worth having language at all...

Sorry for creating the extra churn.

windymilla commented 2 months ago

Good find. I don't see an easy way to fix this without doing a fair amount of introspection on the data on the site search page itself. Right now you can search for parts of languages "eng", "english", "engli" and they all work to search, but will all fail if you refine it. I think we should either accept this limitation or remove language as something you can search on with the site search (which is my preference as a project language isn't going to be highly selective). @bpfoley you requested this feature, what do you think?

If we decide to keep language, I don't think that limitation is too serious, but I agree language is less useful than the others, so I'd vote to remove it. I suppose you could consider it a problem with the Project Search/Refine page that you can give it a URL that it can't reflect in the search form, e.g. https://www.pgdp.org/~cpeel/c.branch/site-search/tools/search.php?show=search&language[]=f lists all projects that have f in the language, but the search form can't reflect that. However, I doubt that it's something the users will notice or need. A workaround is to just re-select the language when using the form to refine.

srjfoo commented 2 months ago

1: If you put in something that it doesn't understand (like ab) you get a warning message. Then do a sensible search (like ah). Then go back to the previous page, you get to the page with the warning message. It seems like it would be better to get back to the raw page or have 'ah' filled in.

I agree this is not ideal. To do that we would need to not ever put the failed search term in the search box and at least right now I think that's the better trade-off. It's quite possible that I'm wrong and we should just never put the failed search term in the box and if the consensus (between the devs in this PR or users post-deploy) is that we don't then I'm happy to change that.

The first thing that came to mind to test this on other sites was archive.org. I tried their advanced search (which I know uses keywords) and set up a search that I knew would work, then changed it slightly to get a null result. Then fixed it again. Going back to the previous page takes you back to the unsuccessful result, so I don't think that's the wrong approach.

2: If you search for a language without capitalizing the first letter, the search understands the language correctly but if you then then do a refine search, the search does not remember the language.

Good find. I don't see an easy way to fix this without doing a fair amount of introspection on the data on the site search page itself. Right now you can search for parts of languages "eng", "english", "engli" and they all work to search, but will all fail if you refine it. I think we should either accept this limitation or remove language as something you can search on with the site search (which is my preference as a project language isn't going to be highly selective). @bpfoley you requested this feature, what do you think?

I played with this, too. If you do a title search, and refine the search, the text you typed in is in the project search form. I assume that the difference is that title (also author) is a text field, and language is a multi-select?

That being said, I agree with you, Brian and Nigel. I'd just remove it. I would think that language would be best used in conjunction with something else (like all German projects available for PP, for instance) and that's just easiest done from the search form itself.

<tb>

Rambling thoughts: As I was doing this round of testing it occurred to me to ask if we should allow a search on PG number (which would point to the project page for that project). Unlike searching for a project by projectID, that would require a keyword. I don't feel strongly about it, though. I would not even expect a search on projectid to be generally useful unless someone is working with a list of bare projectIDs, and (for those of us who might be working from a list of bare projectIDs, yes. But even then, you're better off copying the list of projectids and pasting it into a project search form, because you can give that form multiple projectIDs instead of doing them one at a time.

cpeel commented 2 months ago

language: prefix removed, "Search" name is now also a link and both the icon and the name use pointers for cursors. Addressed other PR feedback.

srjfoo commented 2 months ago

See the image below. The pointer seems to work for the search icon with no problem, and I tried both French and German, and while I was able to keep the pointer from working on the others occasionally, it's not so bad, but "Mitteilungen" is long enough that there's a lot of empty space within the rectangle that shows as a pointer, but doesn't actually bring up the link, both to the left, where the pointer in the image, below, is, and to the right between the number and the label (I assume that it would mirror the left if there weren't a number there.

image

Looking more closely (literally, zooming in on the page header), and clicking around all of the icons (back in English), I'm seeing some inconsistency. I couldn't get the pointer with no link for the wiki or blog icons, but I could for Inbox, Forums and Help. The search icon is a bit different, and clicking anywhere in the enclosing rectangle triggers it just fine.

(I apologize for the image quality, but while I have great fun doing screenshots in general, I can't seem to make them show the cursor, even with that option turned on, so I took a pic with my phone to get this.)

70ray commented 2 months ago

I suppose you could consider it a problem with the Project Search/Refine page that you can give it a URL that it can't reflect in the search form, e.g. https://www.pgdp.org/~cpeel/c.branch/site-search/tools/search.php?show=search&language[]=f lists all projects that have f in the language, but the search form can't reflect that. However, I doubt that it's something the users will notice or need. A workaround is to just re-select the language when using the form to refine.

Yes, this is really a problem with the Project Search page, possibly fixable later. So I think we should keep the language option.

chrismiceli commented 2 months ago

Minor UX feedback, the error message doesn't stand out that much. With most of the page being red, it was difficult to see what went wrong image

cpeel commented 2 months ago

Minor UX feedback, the error message doesn't stand out that much. With most of the page being red, it was difficult to see what went wrong

These are using the existing warning style for paragraphs and our standard style for <kbd> tags to indicate keyboard input. We could style the "don't know what to do with the search" text differently (error type seems a little dramatic for this -- neither warning or error seem entirely accurate) and/or we could change the <kbd> style across the site. The former is probably easier to do as I don't really want to revisit the <kbd> style across the site. Thoughts?

cpeel commented 2 months ago

Looking more closely (literally, zooming in on the page header), and clicking around all of the icons (back in English), I'm seeing some inconsistency.

I've revised the "how things link" and styling once again and now the icons and texts are links and as links are the only thing that show the pointer icon.

I hope this also resolves @chrismiceli 's comment about breaking tabbing as now everything in the header -- including search -- is a link.

Yes, this is really a problem with the Project Search page, possibly fixable later. So I think we should keep the language option.

We can re-add the language option if/when we fix the project search. Easier to add things we know work than release things that don't quite work and field questions.

srjfoo commented 2 months ago

I just realized that I do have another question. The screenshot is from macOS with the system accent color set to hot pink (temporarily, I assure you!). At which point, I realized that I'd been seeing the same effect with my original accent color.

image

Notice that the bolder border around the search box is actually doubled: The default border, as well as the OS accent color. I don't suppose there's a way to get rid of the latter? It's not really needed.

cpeel commented 2 months ago

I don't suppose there's a way to get rid of the latter? It's not really needed.

We're not doing anything "special", that's just the browser indicating that the input field has focus. I don't know of anything that could change it but my CSS-foo is not amazing.

srjfoo commented 2 months ago

I don't suppose there's a way to get rid of the latter? It's not really needed.

We're not doing anything "special", that's just the browser indicating that the input field has focus. I don't know of anything that could change it but my CSS-foo is not amazing.

Based on what other sites do, there is definitely a way to get rid of it (for instance this text input box has a blue highlight while the field is active; phpbb on PMs of forum posts has no special highlighting at all. But like you, I have no idea how to do it. It's not the end of the world, just a bit more emphasis than I think the field really needs. And I just checked -- it's not nearly as much in-your-face on the lighter themes. 🤣

bpfoley commented 2 months ago

It could be cool if we could autocomplete some of the jump options? Definitely a nice to have https://jsfiddle.net/7jdzntke/

Agreed. I was thinking this too, but forgot to bring it up.

70ray commented 2 months ago

I don't suppose there's a way to get rid of the latter? It's not really needed.

We're not doing anything "special", that's just the browser indicating that the input field has focus. I don't know of anything that could change it but my CSS-foo is not amazing.

Based on what other sites do, there is definitely a way to get rid of it (for instance this text input box has a blue highlight while the field is active; phpbb on PMs of forum posts has no special highlighting at all. But like you, I have no idea how to do it. It's not the end of the world, just a bit more emphasis than I think the field really needs. And I just checked -- it's not nearly as much in-your-face on the lighter themes. 🤣

You can remove it with something like "#search-menu-input:focus { outline: none; }" We can't use outline yet but can use its components. https://developer.mozilla.org/en-US/docs/Web/CSS/:focus explains why you shouldn't do this.

https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible describes another way we can't use yet.

cpeel commented 2 months ago

You can remove it with something like

Oh nice, thanks Ray. My inclination is to leave it as-is and not mess with browser accessibility features.

srjfoo commented 2 months ago

You can remove it with something like

Oh nice, thanks Ray. My inclination is to leave it as-is and not mess with browser accessibility features.

I'll just live with it, then.

bpfoley commented 2 months ago

You can remove it with something like

Oh nice, thanks Ray. My inclination is to leave it as-is and not mess with browser accessibility features.

Agreed. The focus ring is intended as a cue for users who use keyboard navigation -- whether it's built-in OS/browser accessibility features, or site-specific Javascript. We should keep it as is unless we've a specific reason to remove it.

srjfoo commented 2 months ago

You can remove it with something like

Oh nice, thanks Ray. My inclination is to leave it as-is and not mess with browser accessibility features.

Agreed. The focus ring is intended as a cue for users who use keyboard navigation -- whether it's built-in OS/browser accessibility features, or site-specific Javascript. We should keep it as is unless we've a specific reason to remove it.

I will defer to wiser heads; I'll get used to it. I do want to mention that, at least for this search, I'm seeing a double ring in all but Safari (though I haven't looked at other browsers/OSs). Who knew that accent colors could be such a pain? Too brigh in this case, not bright enough when highlighting small pieces of text in some cases, with the same base color!

cpeel commented 2 months ago

Rebased to squash the commits and will merge after the CI passes.

Thanks to everyone for your great feedback!