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 links in "See also" section of the API docs. #291

Merged
merged 7 commits into from
Mar 28, 2024

Conversation

ParmarKrishna
Copy link
Contributor

@garg3133 Review Requested.

I have been investigating to find any hard-coded redirect but there are none and entire docs have been updated with /guide.

Afterwards, I found that there is if-else logic running for "See Also" section at, nightwatch-docs/src/includes/api-method.ejs

In that, the line, <li><code><a href="<%- appSettings.baseUrl.replace(/\/$/, '') -%>/api/<%- link.split('.').join('/') %>.html"><%= link %></a></code></li> generates link accordingly.

Now, after many workarounds, I cannot seem to budge the extra two dots that, appSettings.baseUrl.replace(/\$/,'') is appending after ".org" domain.

I tried:
[-] Slicing after replace with (0,-2), but instead of removing double dots, it gives out: https://nightwatchjs.o../
[-] Writing new logic: Gives out same result

I found that, replace() method is creating the problems. Simply removing it, works. Additionally, I added "/" in the BASE_URL in postdoc config.

Do tell me if this is not right, or should I seek another way to solve the problem?

@garg3133 Review Requested.

I have been investigating to find any hard-coded redirect but there are none and entire docs have been updated with `/guide`.

Afterwards, I found that there is if-else logic running for "See Also" section at, `nightwatch-docs/src/includes/api-method.ejs`

In that, the line, `<li><code><a href="<%- appSettings.baseUrl.replace(/\/$/, '') -%>/api/<%- link.split('.').join('/') %>.html"><%= link %></a></code></li>` generates link accordingly.

Now, after many workarounds, I cannot seem to budge the extra two dots that, `appSettings.baseUrl.replace(/\$/,'')` is appending after ".org" domain.

I tried:
[-] Slicing after replace with (0,-2), but instead of removing double dots, it gives out: https://nightwatchjs.o../<other-parts-of-url>
[-] Writing new logic: Gives out same result

I found that, replace() method is creating the problems. Simply removing it, works. Additionally, I added "/" in the BASE_URL in postdoc config.

Do tell me if this is not right, or should I seek another way to solve the problem?
@garg3133
Copy link
Member

@ParmarKrishna Your solution does not seem to be working for me. Maybe you can try removing the complete baseUrl itself from the href attribute and just start the link with /api/?

@garg3133 Review Requested.

As per your suggestion, removing entire appSettings.baseUrl works. Pages seem to be redirected with directly `api`
@ParmarKrishna
Copy link
Contributor Author

@garg3133 Review Requested.

As per your suggestion, removing entire baseUrl does work fine, for all the "See Also" sections.

I did:

  • Remove baseUrl from the line <li><code><a href="<%- appSettings.baseUrl.replace(/\/$/, '') -%>/api/<%- link.split('.').join('/') %>.html"><%= link %></a></code></li> at, nightwatch-docs/src/includes/api-method.ejs

It works. Do let me know if this solution works for you, or should I look for other ways?

PR #291 has been pushed with the changes.

Copy link
Member

@garg3133 garg3133 left a comment

Choose a reason for hiding this comment

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

Looks good. Can you remove all the unnecessary changes from the PR?

postdoc.config.js Outdated Show resolved Hide resolved
@garg3133 garg3133 changed the title #289 Bug Fix Fix links in "See also" section of the API docs. Mar 28, 2024
@garg3133
Copy link
Member

The PR title should contain a brief on what this PR does instead of just writing the issue number.

Copy link
Member

@garg3133 garg3133 left a comment

Choose a reason for hiding this comment

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

Looks good!

@garg3133 garg3133 merged commit 2eb2f9a into nightwatchjs:versions/3.0 Mar 28, 2024
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.

3 participants