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 tags to links in docs #2183

Merged
merged 8 commits into from
Mar 27, 2023
Merged

Add tags to links in docs #2183

merged 8 commits into from
Mar 27, 2023

Conversation

itsyme
Copy link
Contributor

@itsyme itsyme commented Feb 23, 2023

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:

Overview of changes:
Resolves #2179. Added tags to links in docs to differentiate between when both user guide and developer guide are being built together and when only user guide or developer guide is being built alone.

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Add tags to links in docs

Links in docs referencing other sections of MarkBind are all
written in the intra-link form.

This causes confusion when serving the user guide or developer
guide individually as the files to these intra-links are not accessible.

Let's differentiate the links with tags based on the environment,
having intra-links for when the user guide and developer guide are
served together and absolute links for when the user guide and
developer guide are served seperately.

This allows the right behaviour for both situations and creates an
illusion that the user guide and developer guide exist together even
when served separately.


Checklist: ☑️

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

@itsyme itsyme marked this pull request as ready for review March 23, 2023 11:03
@tlylt
Copy link
Contributor

tlylt commented Mar 23, 2023

Thanks @itsyme, could you help to double-check and fix all instances of the existing absolute links from UG to DG (and from DG to UG), such as this one that's not yet updated:
image

@tlylt
Copy link
Contributor

tlylt commented Mar 24, 2023

@itsyme can you add the reasoning in the commit message pls. This PR a bit weird without context.

@tlylt tlylt requested a review from a team March 24, 2023 08:24
Copy link
Contributor

@lhw-1 lhw-1 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @itsyme!

Some questions and nits for the changes, otherwise looks good!

@@ -55,7 +55,7 @@ Ensure that you can serve the MarkBind documentation site in development mode. Y

In the following steps, we will be fixing a hypothetical typo. You do not need to commit the changes or raise a PR, but understand the context and follow the steps to get a feel for the process.

**!!Let's suppose we want to change the use of "Pop-Up" in our [user guide](../../userGuide/components/popups.html) to "Popup" (i.e. remove the hyphen).!!**
**!!Let's suppose we want to change the use of "Pop-Up" in our <a tags="environment--combined" href="/userGuide/components/popups.html">user guide</a><a tags="environment--dg" href="https://markbind.org/userGuide/components/popups.html">user guide</a> to "Popup" (i.e. remove the hyphen).!!**
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are switching to the new combined tag, do we still need the anchor with tags="environment --dg", and if so, in what situations do we need them?

^ This comment applies for all occurrences of this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @lhw-1! We are not exactly switching to the new combined tag, we are adding a new combined tag to differentiate when using serving the site.json which displays both the ug and dg together and serving dg-site.json and ug-site.json individually as the individual dg and ug do not have access to the intra-linked files between them

Copy link
Contributor

@tlylt tlylt Mar 25, 2023

Choose a reason for hiding this comment

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

Hi @lhw-1! We are not exactly switching to the new combined tag, we are adding a new combined tag to differentiate when using serving the site.json which displays both the ug and dg together and serving dg-site.json and ug-site.json individually as the individual dg and ug do not have access to the intra-linked files between them

👍 so there are three scenarios:

  • DG + UG - markbind-master via netlify
  • DG - markbind.org via github pages
  • UG - markbind.org/devdocs via github pages

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @lhw-1! We are not exactly switching to the new combined tag, we are adding a new combined tag to differentiate when using serving the site.json which displays both the ug and dg together and serving dg-site.json and ug-site.json individually as the individual dg and ug do not have access to the intra-linked files between them

Yep, that makes sense!

I think that just as a side note, the duplication of the anchor tags seem unnecessary - a method of declaring an alternate syntax for tags that do not provide such a way could be something to look into. For instance, it would be nice to be able to declare <a tags="environment--combined" href="/userGuide/components/popups.html">user guide</a><alt tags="environment--dg" href="https://markbind.org/userGuide/components/popups.html" />.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that makes sense!

I think that just as a side note, the duplication of the anchor tags seem unnecessary - a method of declaring an alternate syntax for tags that do not provide such a way could be something to look into. For instance, it would be nice to be able to declare <a tags="environment--combined" href="/userGuide/components/popups.html">user guide</a><alt tags="environment--dg" href="https://markbind.org/userGuide/components/popups.html" />.

I think thats a great idea! Do you want to open an issue to request that feature?

docs/site.json Show resolved Hide resolved
@itsyme itsyme changed the title Fix broken links in docs Add tags to links in docs Mar 25, 2023
@itsyme itsyme requested a review from tlylt March 26, 2023 02:09
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.

LGTM @itsyme, try merging it when you are ready, using squash commit

@tlylt tlylt added this to the v4.1.1 milestone Mar 26, 2023
@itsyme itsyme merged commit 8b9cd00 into MarkBind:master Mar 27, 2023
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.

Broken links detected for the deployment to devdocs
3 participants