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

Added IDs to headings for anchor links #1880

Merged
merged 2 commits into from Oct 25, 2023

Conversation

yuliyang
Copy link
Contributor

Lesson plan's pattern template has "Lesson Wrap Up" section at the end that anchor links to Exercises and Assessment.
Lots of lesson plan's anchor links are not working. Initially, the anchor links are #Exercises and #Assessment, so I made all heading's IDs capitalized as well. Let me know if I should change everything to lowercase.

Examples of headings have lowercase IDs but anchor link is capitalized:
https://learn.wordpress.org/lesson-plan/how-to-build-low-code-block-patterns/
https://learn.wordpress.org/lesson-plan/anonymizing-information-in-the-browser/

Example of no IDs in headings:
https://learn.wordpress.org/lesson-plan/how-to-add-and-remove-logo-and-site-icon-in-site-editor/#Assessment

@jonathanbossenger
Copy link
Collaborator

jonathanbossenger commented Oct 10, 2023

I am adding this context from our dev-squad triage discussions, as well as from the training team meeting on 10 October 2023

Generally, when implementing internal anchor link fragments (where an HTML anchor tag links to the id attribute of another element on the same page instead of an external URL) the fragment uses lowercase, not sentence case. While this does not seem to be a requirement according to the HTML spec, it's not something commonly used, as most developers will use lowercase when defining id attributes of HTML elements.

However, the Tip box in the lesson plan saved pattern defines the links as sentence-case, but there are also some instances where it might be all lowercase. So while we're not sure that the case ultimately matters, we do think it should be formalized, documented, and then used accordingly across the site.

So this leaves us with a question, and a follow-up problem:

  1. Should we stick to sentence case, or switch to all lowercase.
  2. Whatever we decide, once implemented, we'll need to review both the lesson plan saved pattern, and any existing lesson plans, to ensure the links work as expected.

@jonathanbossenger
Copy link
Collaborator

@Piyopiyo-Kitsune asked "From my end, I'm curious if this will still be an issue if we consolidate our content like how the new information architecture for Learn proposes :thinking_face: (edited)" with a follow up that "I have a strong feeling that area could change with this restructuring."

If this is the case, then perhaps it just makes more sense to merge this PR as is, and update any broken lesson plans on a case by case basis.

@jonathanbossenger
Copy link
Collaborator

I'm going to add that I think it's probably okay to merge this PR as is.

@jonathanbossenger
Copy link
Collaborator

@adamwoodnz could I ask you to review this, please? Everything looks okay from my end, but I'd prefer a second pair of eyes.

@jonathanbossenger
Copy link
Collaborator

Closes #1919

@jonathanbossenger jonathanbossenger added hacktoberfest To mark issues for the Hacktoberfest event each October. hacktoberfest-accepted labels Oct 12, 2023
Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @yuliyang, nice work!

Let me know if I should change everything to lowercase.

👍 Yes I think we should update this to be more inline with standard practice.

Given this is just a template it won't affect any existing lesson plans. They will need to be edited to match.

<!-- /wp:heading -->

<!-- wp:paragraph -->
<p>There should be one assement item (or more) for each objective listed above. Each assessment item should support an objective; there should be none that don't.</p>
<p>There should be one assessment item (or more) for each objective listed above. Each assessment item should support an objective; there should be none that don't.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@yuliyang
Copy link
Contributor Author

@adamwoodnz I updated all IDs to lowercase and also the anchor links.
I'm not sure how to deal with other languages' pattern template since I can't read them. Can we create a new issue and request for help?

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@adamwoodnz adamwoodnz merged commit 01e2df3 into WordPress:trunk Oct 25, 2023
1 check passed
@adamwoodnz
Copy link
Contributor

adamwoodnz commented Oct 25, 2023

Can we create a new issue and request for help?

German issue created, still need issues for Greek and Hindi.

Would you be able to duplicate the German one please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Dev] Needs Review Pull request needing a review. hacktoberfest To mark issues for the Hacktoberfest event each October.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants