Add tabs for search for better information access#45055
Add tabs for search for better information access#45055bors merged 1 commit intorust-lang:masterfrom
Conversation
|
|
Arf, forgot tidy check once again... |
killercup
left a comment
There was a problem hiding this comment.
Nice! This is a very nice step to making the search more accessible.
At RustFest, we talked a bit about what the search can already do, but one thing I didn't think of: Can you search for a function that has "foo" in its name that returns a usize? I'm not sure I have a good idea how to do the UI for this, but I guess the code itself might already support this in some capacity.
There was a problem hiding this comment.
Heh, that's such a negative way to write this ;) How about
obj && obj.type && obj.type.inputs && obj.type.inputs.length > 0There was a problem hiding this comment.
I wanted to avoid putting all my code in an if condition but either way is fine for me so let's go for it!
There was a problem hiding this comment.
You replace the whole for thing with
return obj.type.inputs.some(function (x) { return x.name === name; })but I'm not sure if we still need to support browsers which don't have Array.prototype.some.
There was a problem hiding this comment.
That's the point. I'm supporting as much browsers as I can.
There was a problem hiding this comment.
Maybe
As return value
for consistency?
There was a problem hiding this comment.
You can look for multiple parameters at once (String, usize -> *).
There was a problem hiding this comment.
Protip:
elems[2].onclick = function() { printTab(2); };can be
elems[2].onclick = printTab.bind(null, 2)There was a problem hiding this comment.
I find it less readable. :-/
dbff8b7 to
52bcc43
Compare
You can't do both at once. You can say that you're looking for a function called Or for a function which returns |
There was a problem hiding this comment.
Forgive my ignorance of the JS part of rustdoc, but i think i'm missing something. What is obj.type here, and why does its presence keep it from getting into results['others']? Is that on purpose?
There was a problem hiding this comment.
You can see everything as a hashmap in js, so obj.type == obj['type']. In this case, if there is no type (which is just a regular field, nothing particular), it means it's not a function nor a method so I put it in "others".
There was a problem hiding this comment.
So is results['others'] not meant to be the same as the current search results? I'm concerned that you won't be able to find a function by name if it's not returning something with the same name. For example, are you able to find str::to_lowercase by searching for something like lowercase? Will you be able to find that method on any tab in your new layout?
1cb08c0 to
8a53447
Compare
Make tabs work
8a53447 to
3a65d12
Compare
| } | ||
| } | ||
| if (results['others'].length < maxResults && | ||
| ((query.search && obj.name.indexOf(query.search)) || added === false)) { |
There was a problem hiding this comment.
Why is this condition not just results['others'].length < maxResults? I just checked out your PR and didn't see any difference between the longer form you have here and taking these extra conditions out. What is this trying to do?
There was a problem hiding this comment.
In case the only thing(s ?) match are the arguments or the returned type, I don't want to add extra information. For me, the 'others' tab is also about the type/function/module's name so it seems logical to not add everything (even if the filter is pretty light).
There was a problem hiding this comment.
After some checking, it seems like it won't stop things getting into results if their name matches as well as having a parameter type that matches (e.g. "Result" will show Try::into_result on both the "As return value" tab as well as the "Types/modules" tab) so i'll retract this concern.
|
@bors r+ I'm incredibly excited to see this land. I think it greatly helps discoverability in docs. It's not quite perfect (e.g. searches for io things won't get a lot of type-based results since they're all wrapped up in cc #44024 since this helps that out |
|
📌 Commit 3a65d12 has been approved by |
|
⌛ Testing commit 3a65d12 with merge 8dcd0e127953f5eaff18aad9f1ea54f4c9712d02... |
|
💔 Test failed - status-travis |
|
@bors retry
|
Add tabs for search for better information access A few screenshots: <img width="1440" alt="screen shot 2017-10-06 at 00 54 51" src="https://user-images.githubusercontent.com/3050060/31256148-032c1a06-aa31-11e7-8e4c-fec59786b8e6.png"> <img width="1440" alt="screen shot 2017-10-06 at 00 54 58" src="https://user-images.githubusercontent.com/3050060/31256150-03312cb2-aa31-11e7-86f7-8c9f0d8d6d4f.png"> <img width="1440" alt="screen shot 2017-10-06 at 00 55 00" src="https://user-images.githubusercontent.com/3050060/31256149-0330d456-aa31-11e7-8f89-3b3c824e30b4.png"> r? @rust-lang/docs cc @killercup @QuietMisdreavus
|
☀️ Test successful - status-appveyor, status-travis |
A few screenshots:
r? @rust-lang/docs
cc @killercup @QuietMisdreavus