-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
improve HTML title on repositories #27020
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
Conversation
- The `<title>` element that lives inside the `<head>` element is an important element that gives browsers and search engine crawlers the title of the webpage, hence the element name. It's therefor important that this title is accurate. - Currently there are three issues with titles on repositories. It doesn't use the `FullName` and instead only uses the repository name, this doesn't distinguish which user or organisation the repository is on. It doesn't show the full treepath in the title when visiting an file inside a directory and instead only uses the latest path in treepath. It can show the repository name twice if the `.Title` variable also included the repository name such as on the repository homepage. - Use the repository's fullname (which include which user the repository is on) instead of just their name. - Display the repository's fullname if it isn't already in `.Title`. - Use the full treepath in the repository code view instead of just the last path. - Adds integration tests. - Adds a new repository (`repo59`) that has 3 depths for folders, which wasn't in any other fixture repository yet, so the full treepath for could be properly tested. - Resolves https://codeberg.org/forgejo/forgejo/issues/1276 (cherry picked from commit 8712eaa394053a8c8f1f4cb17307e094c65c7059)
8606fb9 to
34555b1
Compare
This path shortening is intentional, otherwise with deeply nested files, the tab title would become unreadable if all you see is directory names, given that usually there is only around ~20-50 characters available on a tab title. This behaviour matches GitHub and I want to keep it. |
routers/web/repo/view.go
Outdated
| defer dataRc.Close() | ||
|
|
||
| ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+path.Base(ctx.Repo.TreePath), ctx.Repo.RefName) | ||
| ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+util.PathEscapeSegments(ctx.Repo.TreePath), ctx.Repo.RefName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the code is wrong: do not do double-escaping.
I also agree to keep the short title, do not show the full path. For example, many languages like Java/PHP have quite deep paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly GitHub shows the full path currently when viewing files, but I assume that is a regression from their recent react rewrite of the file view, not an intentional change. In any case I prefer the current short paths.
Original short title was added in #17987.
Refs: https://codeberg.org/forgejo/forgejo/pulls/1423 (cherry picked from commit 63e99df)
<title>element that lives inside the<head>element is an important element that gives browsers and search engine crawlers the title of the webpage, hence the element name. It's therefor important that this title is accurate.FullNameand instead only uses the repository name, this doesn't distinguish which user or organisation the repository is on. It doesn't show the full treepath in the title when visiting an file inside a directory and instead only uses the latest path in treepath. It can show the repository name twice if the.Titlevariable also included the repository name such as on the repository homepage..Title.repo59) that has 3 depths for folders, which wasn't in any other fixture repository yet, so the full treepath for could be properly tested.(cherry picked from commit 8712eaa394053a8c8f1f4cb17307e094c65c7059)