Skip to content

Conversation

@ang-zeyu
Copy link
Contributor

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

See #1955 (comment)

Overview of changes:

  • Use absolute URLs for the asset files

Anything you'd like to highlight/discuss:
na

Testing instructions:
na

Proposed commit message: (wrap lines at 72 characters)
Use absolute URLs for assets


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

@ang-zeyu ang-zeyu force-pushed the use-absolute-path-assets branch from 91a407a to 5f9a99e Compare September 22, 2022 16:00
<script src="../../markbind/js/polyfill.min.js"></script>
<script src="../../markbind/js/vue.min.js"></script>
<script src="../../markbind/js/markbind.min.js"></script>
<link rel="stylesheet" href="/test_site/markbind/css/bootstrap.min.css">
Copy link
Contributor

@tlylt tlylt Sep 22, 2022

Choose a reason for hiding this comment

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

Is this a concern? I am thinking maybe it needs to read the files under the nested_sub_site, which maybe somehow could be different from the outer site (maybe due to a change in markbind version).

(hmm is subsite supposed to contain its own stylesheets etc?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, subsites also use the parent site's assets.

I understand the confusion; I don't think we're clearly communicating enough what subsites really are. (I was confused in the beginning as well, also I see it many times in #1870)

At the current its just a over the top way of saying: I will ensure your use of {{ baseUrl }} in subsites resolves correctly, if you try to use a file in the subsite from the parent site.

"Use" being anything that requires resolving these links correctly:

  • <include/panel>-ing it
  • {% include %}-ing it
  • probably more

I've come to think the status quo is fine (but the documentation clearly needs to be made clearer given all the confusion), but if you have any ideas please help to drive the discussion #1307! 🙌

cc @damithc

Copy link
Contributor

Choose a reason for hiding this comment

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

In the CS2103 website, se-book has it's own assets which are used when it is on its own e.g., https://se-education.org/se-book/
But when the se-book is being used as a sub-site inside CS2103 website, the outer site's assets are being used for the se-book as well (I think).

Copy link
Contributor

@tlylt tlylt left a comment

Choose a reason for hiding this comment

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

Thanks @ang-zeyu , the updated paths seem to work as expected after the change. I think there's a preference for relative path over absolute path, but I am not sure whether there's a preference of paths relative to root vs relative the current folder, any thoughts?

@tlylt tlylt added this to the 4.0.3 milestone Sep 24, 2022
@ang-zeyu ang-zeyu merged commit 76a2462 into MarkBind:master Sep 25, 2022
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