Skip to content
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

Limit page title to 60-70 chars for SEO #677

Closed
wants to merge 2 commits into from

Conversation

nickdenardis
Copy link
Member

@nickdenardis nickdenardis commented Nov 27, 2023

Reason for change

When page titles are longer, the addition of the site name, sur-title name, and then "Wayne State University" pushes the title tag into the triple digits for length.

SEO recommends 60-character limit for page titles.

Proposed solution

Because the page title can be changed up to the line before the view is called, the logic needs to exist in the template itself. Dropping right into raw PHP in Blade.

Moved the logic to a helper function which is called within the merge() function before the view.

Tests have also been added.

  • The page title from the CMS is not modified and is prioritized
  • The sur-title site name has been dropped, it isn't needed in this context since it is already visible on the page
  • The site title is optional based on space. From all the recommendations we have seen, this is less important than the university name
  • University name is included only if there is enough room and doesn't push it over an extended 70-char limit

Examples

https://engineering.wayne.edu/mechanical/academics/bs
Current: <title>Bachelor of Science in mechanical engineering - Mechanical Engineering - Wayne State University</title>
Ideal: <title>Bachelor of Science in mechanical engineering - Wayne State University</title>

https://engineering.wayne.edu/electrical-computer/academics/master-electrical-engineering
Current: <title>Master of science in electrical engineering - Electrical and Computer Engineering - Wayne State University</title>
Ideal: <title>Master of science in electrical engineering - Wayne State University</title>

TODO

  • Logic in place
  • Tests in place

@nickdenardis nickdenardis self-assigned this Nov 27, 2023
@nickdenardis nickdenardis marked this pull request as draft November 27, 2023 19:19
@chrispelzer
Copy link
Member

chrispelzer commented Nov 27, 2023

If you didn't want to do it in the template, you could also move it to the merge() function which should be part of each of the Controller on it's way to the view.
https://github.com/waynestate/base-site/blob/master/app/Support/helpers.php#L8-L39

Also can be tested with where the merge tests happen, though not 100% sure.

Long-term solution, we move the 2 helper functions to part of a helper repository for each of them as a class rather than a functions.

@nickdenardis
Copy link
Member Author

If you didn't want to do it in the template, you could also move it to the merge() function which should be part of each of the Controller on it's way to the view. https://github.com/waynestate/base-site/blob/master/app/Support/helpers.php#L8-L39

Also can be tested with where the merge tests happen, though not 100% sure.

Long-term solution, we move the 2 helper functions to part of a helper repository for each of them as a class rather than a functions.

That is not a bad idea! All of $request->data looks like it is there and it would be easier to test.

@coveralls
Copy link

coveralls commented Nov 27, 2023

Coverage Status

coverage: 99.906% (+0.002%) from 99.904%
when pulling 8293c10 on feature/page-title-seo
into 6ad35d7 on develop.

@nickdenardis nickdenardis marked this pull request as ready for review November 29, 2023 13:31
Copy link
Member

@breakdancingcat breakdancingcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 Thank you!

@nickdenardis nickdenardis deleted the feature/page-title-seo branch November 29, 2023 18:26
Copy link
Member

@chrispelzer chrispelzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me, lot cleaner

@breakdancingcat breakdancingcat mentioned this pull request Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants