Skip to content

Crate name / search term should be in the page title #1930

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

Closed
upsuper opened this issue Nov 28, 2019 · 6 comments
Closed

Crate name / search term should be in the page title #1930

upsuper opened this issue Nov 28, 2019 · 6 comments
Assignees
Labels
A-frontend 🐹 C-bug 🐞 Category: unintended, undesired behavior E-help-wanted

Comments

@upsuper
Copy link

upsuper commented Nov 28, 2019

When I'm looking for some crates, I end up getting several crates.io pages which basically look the same from the title, which is unfortunate.

crates-io-screenshot

I suggest that the page title should include the crate name / search term etc., and ideally at the beginning of the page title, which would make it easier to find the desired page.

@smarnach
Copy link
Contributor

smarnach commented Nov 28, 2019

I agree with this suggestion. I think a title in the form "<search term> · Search results · crates.io" or "<crate name> · crates.io" would make sense. This is in line with what Wikipedia and many other web pages do.

@smarnach smarnach added A-ui C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works E-help-wanted labels Nov 28, 2019
@hbina
Copy link
Contributor

hbina commented Nov 30, 2019

Any related files that I can look at?

@phlopsi
Copy link

phlopsi commented Dec 1, 2019

Someone changed the title generation over the course of the last two (?) months. Previously, when I bookmarked a crate page it'd always show, e.g. "serde - Cargo: packages for Rust", "lalrpop - Cargo: packages for Rust", etc. The current title being the same for every crate ("crates.io: Rust Package Registry") is a clear feature regression. It's annoying, that I have to edit every bookmark to contain the crate name, now. Rather than an enhancement, this should be treated as a bug, because we already had this feature and someone removed it.

If this feature gets re-implemented I'd suggest, that a should be test added, that checks, if the crate name is somewhere inside the page title. Clearly, if someone removes a test, it must be noticed and questioned during code review, i.e. a test should prevent any future PRs to remove this feature, again.

@smarnach
Copy link
Contributor

smarnach commented Dec 1, 2019

@phlopsi There has been a change to the branding in #1787, but this really only changed the strings from "Cargo" to "crates.io". But it is true that this used to work correctly, and we even have a test for it that still passes. Clearly something has regressed, so I will tag this as a bug instead.

@smarnach smarnach added C-bug 🐞 Category: unintended, undesired behavior P-medium and removed C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works P-low labels Dec 1, 2019
@carols10cents
Copy link
Member

Interesting, so I think the tests are passing because of this commit, which was trying to work around Testem adding its own title element. The testem team feels that testing the title should be done at an integration test level.

There are indeed 2 <title> elements on pages like https://crates.io/search?q=test, which I can see by doing document.head.getElementsByTagName('title') in the developer console.

This PR added a generic <title> to fix an issue with ember-page-title that caused NO titles to appear, and the tests for the titles didn't fail then either :(

So I wonder if we need to update ember-page-title? We're currently on 4.0.5 and the latest is 5.1.0.

There's also this interesting note in the README:

Non-fastboot apps should keep the <title> tag in index.html to ensure that the initial page is valid HTML. The title will be removed and replaced when your app boots.

Fastboot apps MUST remove the <title> tag from index.html.

We're, uh, both. So I'm not sure what we need to do here, but it seems like we're doing the wrong thing.

@carols10cents
Copy link
Member

Fixed by #2074, I'm working on deploying this soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend 🐹 C-bug 🐞 Category: unintended, undesired behavior E-help-wanted
Projects
None yet
Development

No branches or pull requests

7 participants