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 the /paths from absolute to relative in HTMLFix paths #10

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

Conversation

karthik-sivasubramaniam
Copy link

@karthik-sivasubramaniam karthik-sivasubramaniam commented Aug 6, 2022

Fixes #5

A few caveats/heads-up/feedback:

  • The #community URL fragment is linked with a target="_blank" attribute, which could still make sense for other pages, but on the home page that the fragment belongs to, it looks weird.
  • There is an odd link that look likes an email but is put under the current directory's path (/2021/12/16/stackstorm-v3-6-0-released/Aaron.jonen@cypherint.com). I left it as it is. Not sure if there are more like that.
  • There are broken links found across the repo (such as /start-now/ and /news-events/) mostly directly below the root level. I have fixed the relative path for them anyway. I haven't found/made a note of all such broken links.
  • There are also a couple other broken links caused by what seem to be obsolete/time-sensitive pages that are no longer there (example: /meeting-registration-tuesday-july-22-1130-pdt at /2014/07/23/stackstorm-devops-automation-interactive-webinars-register-today/index.html. I have fixed their relative paths as well, FWIW.
  • There is a broken reference to a non-existent wp-json directory/file in so many <link> tags across the codebase, but never within an <a> tag. I left it as it is.

@CLAassistant
Copy link

CLAassistant commented Aug 6, 2022

CLA assistant check
All committers have signed the CLA.

@arm4b arm4b added the enhancement New feature or request label Aug 8, 2022
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Thanks a lot for composing this PR!

Looking at the details and how the URLs change, it seems to get a bit messier than expected considering so many levels for relative path change.
The experiment helps a lot to see all the pros/cons, the real diff and I'm not sure anymore if we should go this way. Curious what others @StackStorm/maintainers think.

At a minimum relevant to this PR, we shouldn't try to link to index.html in the new changes.

@@ -416,7 +416,7 @@ <h1 class="entry-title">SCALE 12x</h1>
</div>


<!--googleoff: all--><div id="cookie-law-info-bar" data-nosnippet="true"><span>We use cookies for traffic analytics and ad and content personalization. By clicking on any of the content or interacting with any section of this website,<br>you are agreeing to this use of cookies in the manner described in our <a href="/privacy-policy" id="CONSTANT_OPEN_URL" target="_blank" class="cli-plugin-main-link" style="display:inline-block;">Privacy Policy</a> <a role="button" tabindex="0" data-cli_action="accept" id="cookie_action_close_header" class="cli-plugin-main-button cookie_action_close_header cli_action_button" style="display:inline-block; ">close [x]</a></span></div>
<!--googleoff: all--><div id="cookie-law-info-bar" data-nosnippet="true"><span>We use cookies for traffic analytics and ad and content personalization. By clicking on any of the content or interacting with any section of this website,<br>you are agreeing to this use of cookies in the manner described in our <a href="../../../../privacy-policy/index.html" id="CONSTANT_OPEN_URL" target="_blank" class="cli-plugin-main-link" style="display:inline-block;">Privacy Policy</a> <a role="button" tabindex="0" data-cli_action="accept" id="cookie_action_close_header" class="cli-plugin-main-button cookie_action_close_header cli_action_button" style="display:inline-block; ">close [x]</a></span></div>
Copy link
Member

@arm4b arm4b Aug 8, 2022

Choose a reason for hiding this comment

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

Yeah, that's a lot of levels for the relative path ...

-<a href="/privacy-policy"
+<a href="../../../../privacy-policy/index.html"

@@ -337,57 +337,57 @@ <h1 class="entry-title">[SDX Central] StackStorm Takes A Modernized Shot At DevO
<section id="text-7" class="col"><div class="scn"> <div class="textwidget">
<p><a href="http://docs.stackstorm.com/" target="_blank" rel="noopener">Documentation</a><br>
<a href="https://github.com/StackStorm" target="_blank" rel="noopener">GitHub</a><br>
<a href="/#community" target="_blank" rel="noopener">Community</a><br>
<a href="/security" target="_blank" rel="noopener">Security</a></p>
<a href="../../../../index.html#community" target="_blank" rel="noopener">Community</a><br>
Copy link
Member

Choose a reason for hiding this comment

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

Oh well.
Yeah, I see it's getting messy.

</div>
</div></section> </div>
<div class="colm oth ">
<section id="text-8" class="col"><div class="scn"> <div class="textwidget">
<p><a href="/blog">Blog</a><br>
<p><a href="../../../../blog/index.html">Blog</a><br>
Copy link
Member

Choose a reason for hiding this comment

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

At minimum we shouldn't point to index.html in any case as that part should be hidden

Suggested change
<p><a href="../../../../blog/index.html">Blog</a><br>
<p><a href="../../../../blog">Blog</a><br>

Choose a reason for hiding this comment

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

Makes sense.

The code base is plain HTML/JS/CSS, so I believe there aren't much options for resolving URL paths and for hiding the filename while still having the links work. We could technically create a global variable that yields the root directory's path and share it across all the files using a <script> tag, but that will only make the codebase messier and more difficult to maintain, since every src and href value would need to be injected via JS. Of course, even then the website would break on JavaScript disabled browsers.

If this codebase is a part of some kind of framework when ported to your testing/production environment - something that can handle dynamic values across the files - then I believe the solution should become much simpler.

I'm not yet an experienced developer (just about 2 years under my belt) and this is my first ever open source contribution, so please feel free to correct me wherever I'm wrong. Appreciate your comments. A big thanks for reviewing my contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix the /paths from absolute to relative in HTML
3 participants