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

Add external_urls filter #1495

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

Conversation

MasterEnoc
Copy link
Contributor

This a tiny filter whose purpose is replace urls found in <a> tags for relative URLs pointing to devdocs documentation. This is done manually, each author or maintainer should add (if needed) options[:external_url] filter option to the scraper.

This is not done server-side as discussed in #234 but it gives an option to avoid links of external documentation when it is available in devdocs.

This commit already implements this filter in the backbone.rb file.

This filter traverses all <a> tags and replaces
its url for an url poiting to a path of an existant
documentation.
@MasterEnoc MasterEnoc requested a review from a team as a code owner March 5, 2021 22:46
Copy link
Contributor

@simon04 simon04 left a comment

Choose a reason for hiding this comment

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

Great idea! Please see my comments.

See also: former PR #714 by @j-f1

@@ -84,6 +84,7 @@ The `call` method must return either `doc` or `html`, depending on the type of f
* [`AttributionFilter`](https://github.com/freeCodeCamp/devdocs/blob/master/lib/docs/filters/core/attribution.rb) — appends the license info and link to the original document
* [`TitleFilter`](https://github.com/freeCodeCamp/devdocs/blob/master/lib/docs/filters/core/title.rb) — prepends the document with a title (disabled by default)
* [`EntriesFilter`](https://github.com/freeCodeCamp/devdocs/blob/master/lib/docs/filters/core/entries.rb) — abstract filter for extracting the page's metadata
* [`ExternalUrlsFilter`](https://github.com/freeCodeCamp/devdocs/blob/master/lib/docs/filters/core/external_urls.rb) — replaces external URLs for relative URLs of existant devdocs documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/existant/existing

def call
if context[:external_urls]

root = path_to_root
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we simply use the following?

Suggested change
root = path_to_root
root = '/'


context[:external_urls].each do |host, name|
if url.host.to_s.match?(host)
node['href'] = root + name + url.path.to_s + '#' + url.fragment.to_s
Copy link
Contributor

Choose a reason for hiding this comment

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

The generated link is correct, but does not work in devdocs. Presumably a JavaScript call to app.enableDoc(...) is required.

Screen.Recording.2021-03-13.at.19.44.22.mov

@MasterEnoc
Copy link
Contributor Author

Code improved by your suggestions @simon04.

Reading the js of devdocs I found that each link ('a' tag) is verified if it is part of devdocs or not before send me to the page that it is pointing, if the link is part of devdocs it will make an xhr request and it will show me content retrieved by the xhr request, this behaviour works if the link is part of the same documentation or if the link is from a documentation that is enabled, if not it will show me an error. I changed this behaviour, if a link is part of devdocs but it is from a different doc it will send to devdocs reloading the page rather than make an xhr request.

isSameOriginDifferentDoc = (url) ->
console.log(url.pathname)
console.log(location.pathname)
url.pathname == location.pathname
Copy link
Contributor

Choose a reason for hiding this comment

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

For https://devdocs.io/typescript/declaration-files/do-s-and-don-ts#function-overloads, location.pathname is "/typescript/declaration-files/do-s-and-don-ts".

Remove the console.log statements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

console.log removed and now should work checking only the initial path of the pathname.

Elementes in the sidebar werent served as xhr
request due this feature, now it works checking
if it is an element in the sidebar
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