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

The 0-MOOC and Algorithms and Data Structures anchor tags not working #6508

Closed
Harshita-Kohli opened this issue Oct 29, 2021 · 8 comments · Fixed by #6677
Closed

The 0-MOOC and Algorithms and Data Structures anchor tags not working #6508

Harshita-Kohli opened this issue Oct 29, 2021 · 8 comments · Fixed by #6677
Assignees
Labels
help wanted Needs help solving a blocked / stucked item PR requested Issues that can be addressed with a new PR

Comments

@Harshita-Kohli
Copy link
Contributor

On clicking 0-MOOC and Algorithms and Data Structures on the webpage, we cannot jump to the respective sections on the page. This feature is not working only for these 2 links.
a tags

@Harshita-Kohli
Copy link
Contributor Author

I can solve this issue.

@eshellman eshellman added the PR requested Issues that can be addressed with a new PR label Oct 29, 2021
Harshita-Kohli added a commit to Harshita-Kohli/free-programming-books that referenced this issue Oct 29, 2021
Corrected hrefs of 0-MOOC and Algorithms and Data Structures according to their ids EbookFoundation#6508
@davorpa
Copy link
Member

davorpa commented Oct 31, 2021

It works well on markdown files.

@charlottetan more issues with GFM - Kramdown

@davorpa davorpa added the help wanted Needs help solving a blocked / stucked item label Oct 31, 2021
@davorpa davorpa linked a pull request Jan 23, 2022 that will close this issue
5 tasks
@charlottetan
Copy link
Contributor

charlottetan commented Jan 23, 2022

@davorpa bummer that parsing continues to be an issue!

I spent some time investigating and have two options available:

1. PR #6677 - We switch the parser back to Kramdown (recommended)

We originally switched to GFM in PR #5810, before we realised we could use markdown="1" in the div as suggested by Kramdown in kramdown/parser-gfm#28. I tried out their suggestion and it mostly works. There was one small issue where plain URLs were not automatically parsed, which means we have to write [https://github.com](https://github.com) instead of just https://github.com.

I have my github pages pointed to the branch for this PR and the pages can be viewed here:

And this also results in the anchor tags working:

2. PR #6678 - We modify the anchor tags and set them via HTML

When headings include special characters, instead of writing

### 0 - MOOC
### Algorithms & Data Structures

We do this instead

<h3 id="0---mooc">0 - MOOC</h3>
<h3 id="algorithms--data-structures">Algorithms & Data Structures</h3>

I don't like this approach as much, like you point out in https://github.com/EbookFoundation/free-programming-books/pull/6678/files#r790243208, this requires a lot of extra effort to make sure it's always right. On top of that, there are many files we'd have to go through to change this in. Also sounds like it might not play well with the new search #6643.

Results here:
https://github.com/charlottetan/free-programming-books/blob/charlottetan-try-fix-anchors/courses/free-courses-en.md
No github pages link since it's currently pointing to the other branch.

@davorpa
Copy link
Member

davorpa commented Jan 23, 2022

@davorpa bummer that parsing continues to be an issue!

I spent some time investigating and have two options available:

Thanks for your valuable hard work 🤟 !!!

2. PR #6678 - We modify the anchor tags and set them via HTML

...

At this moment the second is discarded, I thought, due to the reason we post in #6678 (comment) related with #6643. What do you think @EbookFoundation/reviewers ?

@charlottetan
Copy link
Contributor

Yeah I think we should go with PR #6677. I also see more discussion in that PR on other possible permutations of the processor

@eshellman
Copy link
Collaborator

have asked to folks working on the search to comment. Thanks @charlottetan for looking at this.

@brogan20
Copy link
Contributor

Switching to Kramdown (#6677) would be ideal here. #6678 would completely break the search engine and parser used in #6643.

@eshellman
Copy link
Collaborator

@charlottetan Seems to be consensus on #6677 Ready to merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Needs help solving a blocked / stucked item PR requested Issues that can be addressed with a new PR
Projects
None yet
5 participants