Skip to content

Make doc search results use <a> tags instead of js for navigating #16909

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

Merged
merged 1 commit into from
Sep 4, 2014
Merged

Make doc search results use <a> tags instead of js for navigating #16909

merged 1 commit into from
Sep 4, 2014

Conversation

carols10cents
Copy link
Member

This has the primary advantage of not interfering with browser default behavior for links like being able to cmd/ctrl+click on a result to open the result in a new tab but leave the current page as-is (previous behavior both opened a new tab and changed the current tab's location to the result's).

I've done my best to keep the rest of the behavior and the appearance the same-- the whole row still highlights, still has a hand cursor, still moves to the result page with a normal click, arrows+enter still work. If the result is on the current page, the search is simply hidden.

The biggest difference in behavior is that people using tab to navigate through the links will have to hit tab twice for each row, since each cell has its own a tag.. I could fix this by switching to divs and spans instead of a table, but that's potentially more CSS finicky?

The biggest difference in appearance is probably that all the text in the search results is Fira Sans now, instead of just the method name with the rest of the text in Source Serif Pro. I can put this appearance back, but it looks like all links anywhere on the page are Fira Sans. Only the name was in an a tag before, but the whole row was ACTING like a link, so I think this is actually more consistent.

I've pushed these changes to a gh-pages repo if you'd like to take a look at the effects; note that I also have my changes for PR #16735 there too so the search results will be sorted differently than on master.

This has the primary advantage of not interfering with browser default
behavior for links like being able to cmd/ctrl+click on a result to open
the result in a new tab but leave the current page as-is (previous
behavior both opened a new tab and changed the current tab's location to
the result's)
@alexcrichton
Copy link
Member

Sorry for being a little slow on this, this looks great @carols10cents! I personally can't even use tab to navigate today, so I don't think we're losing too much in that department.

bors added a commit that referenced this pull request Sep 4, 2014
This has the primary advantage of not interfering with browser default behavior for links like being able to cmd/ctrl+click on a result to open the result in a new tab but leave the current page as-is (previous behavior both opened a new tab and changed the current tab's location to the result's).

I've done my best to keep the rest of the behavior and the appearance the same-- the whole row still highlights, still has a hand cursor, still moves to the result page with a normal click, arrows+enter still work. If the result is on the current page, the search is simply hidden.

The biggest difference in behavior is that people using tab to navigate through the links will have to hit tab twice for each row, since each cell has its own `a` tag.. I could fix this by switching to `div`s and `span`s instead of a table, but that's potentially more CSS finicky?

The biggest difference in appearance is probably that all the text in the search results is Fira Sans now, instead of just the method name with the rest of the text in Source Serif Pro. I can put this appearance back, but it looks like all links anywhere on the page are Fira Sans. Only the name was in an `a` tag before, but the whole row was ACTING like a link, so I think this is actually more consistent.

[I've pushed these changes to a gh-pages repo](https://carols10cents.github.io/rustdoc-playground/std/index.html?search=t) if you'd like to take a look at the effects; note that I also have my changes for PR #16735 there too so the search results will be sorted differently than on master.
@bors bors closed this Sep 4, 2014
@bors bors merged commit 3dc9deb into rust-lang:master Sep 4, 2014
@carols10cents
Copy link
Member Author

Thank you @alexcrichton!!!! :)

@Manishearth
Copy link
Member

Thanks for this! I had a fix in hand, but I was too lazy to trawl through the CSS :P

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 31, 2024
fix: Keep the span for `Attr::Literal` around
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants