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

fix: Add SEO Page Title for 2 pages #7326

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ajitzero
Copy link

@ajitzero ajitzero commented Aug 19, 2023

Description:

  • The page title is taken from the h1 tag of a page in this component.
    • For "Operator Decision Tree", the h1 tag was placed within the component for the page and not in the HTML file itself. Moved it into the correct file now.
    • For "Team", there was no h1 tag. Added this now.
  • Typo: "Alumn" should be "Alumni". Unrelated to the above, but I noticed it and included it here. I can remove it from here and submit it separately if anyone has strong opinions about that.

Related issue (if exists):
None

Screenshot of fix:

Case Before After
Page title of "Operator Decision Tree" & inline # link image image
Page title of "Team" image image
Typo in "Team" > "Alumni" image image

I suggest squash-committing this since I edited the commit messages via GitHub UI and will not pass commit-lint. Fixed commit messages.

@ajitzero ajitzero changed the title fix: SEO Page Title for Operator Decision Tree fix: Add SEO Page Title for 2 pages Aug 19, 2023
@ajitzero ajitzero marked this pull request as ready for review August 19, 2023 13:21
Copy link
Member

@jakovljevic-mladen jakovljevic-mladen left a comment

Choose a reason for hiding this comment

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

Please revert one of the changes (the second example from the PR description) as it is redundant. And also please merge master branch to resolve conflicts. Otherwise, looks good.

@@ -1 +1,2 @@
<h1 class="no-toc">Team</h1>
Copy link
Member

Choose a reason for hiding this comment

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

I think this heading is not needed. The reason for this is that the heading is still visible in the top header:

image

Please revert this one.

@@ -110,7 +110,7 @@
"github": "https://github.com/mattpodwysocki",
"picture": "https://avatars0.githubusercontent.com/u/49051",
"twitter": "https://twitter.com/mattpodwysocki",
"group": "Alumn"
"group": "Alumni"
Copy link
Member

Choose a reason for hiding this comment

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

As a non-native English speaker, I hope this one is right 🙂 I don't really know. Maybe @benlesh can help?

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.

2 participants