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

Update default template with UG links and useful information #2225

Merged
merged 22 commits into from
Apr 2, 2023

Conversation

lhw-1
Copy link
Contributor

@lhw-1 lhw-1 commented Mar 18, 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:

Addresses a few of the main outcomes of the discussions in #2214, and updates the default template to include links to the main sections of the User Guide. Also possibly addresses #519.

Here is a preview of what it currently looks like:

image

I've kept the new look to be as simple as possible, and have modelled it based partially on the MarkBind User Guide - Authoring Contents section and partially on the Gatsby initial site.

This is just a possible direction we could take the default template towards - if there are any suggestions to improve this current look, or if there are any concerns / issues with this, please highlight as needed!

Anything you'd like to highlight/discuss:

We should keep in mind to keep the template as simple as possible while including the main features that a user using markbind init --convert --template default would want to have in their site. (E.g. siteNav and pageNav are essential for this - any other suggestions are welcome.)

Testing instructions:

Run markbind init in a new directory to initialize a site built using the default template.

Alternatively, you can view the deployed version of the template here.

Proposed commit message: (wrap lines at 72 characters)

Update default template with UG links and useful information

The current default template contains filler content and
component usage that is not well explained.

This could cause confusion to users who are starting with the
default template from scratch for their site.

Updating the default template allows us to help guide the newer
users, and since the content is overriden when being converted,
will not affect users who are converting pre-existing documents.

Let's add links to our User Guide, and re-organize the information
given to the users in a more succinct manner, while also
showcasing MarkBind components.

Checklist: ☑️

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

Copy link
Contributor

@jovyntls jovyntls left a comment

Choose a reason for hiding this comment

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

Template mostly LGTM! I'm not a fan of the empty pagenav on the right (it seems a bit weird to have a pagenav that has a "Chapters of this page" header but no contents) - could we add some unobtrusive headers to populate the pagenav? I think it's still worth showcasing that the pagenav exists since it's one of the key features of the default template.

packages/core/template/default/index.md Outdated Show resolved Hide resolved
Co-authored-by: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
@lhw-1
Copy link
Contributor Author

lhw-1 commented Mar 20, 2023

Template mostly LGTM! I'm not a fan of the empty pagenav on the right (it seems a bit weird to have a pagenav that has a "Chapters of this page" header but no contents) - could we add some unobtrusive headers to populate the pagenav? I think it's still worth showcasing that the pagenav exists since it's one of the key features of the default template.

That's a good point - I'll include more sections from the main MarkBind page to make the site longer (and thus have functional page nav) 👍

@tlylt
Copy link
Contributor

tlylt commented Mar 20, 2023

@lhw-1 maybe set up a site, deploy it, and add the link to the description? Easier to preview that way?

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 @ lhw-1, this revamp is great. Some nits as mentioned.

As much as possible, I think we should use MarkBind features on this page to present the appropriate information. Not suggesting that we go back to the current site where we include different versions of the panel state to showcase the style of the header when expanded or minimized, but use appropriate components to display information. Panels, box, tooltip, etc are all possible.

packages/core/template/default/index.md Outdated Show resolved Hide resolved
@lhw-1
Copy link
Contributor Author

lhw-1 commented Mar 20, 2023

@lhw-1 maybe set up a site, deploy it, and add the link to the description? Easier to preview that way?

Done! I've deployed it and added it to the description (for convenience, link here).

@lhw-1
Copy link
Contributor Author

lhw-1 commented Mar 20, 2023

Template mostly LGTM! I'm not a fan of the empty pagenav on the right (it seems a bit weird to have a pagenav that has a "Chapters of this page" header but no contents) - could we add some unobtrusive headers to populate the pagenav? I think it's still worth showcasing that the pagenav exists since it's one of the key features of the default template.

As much as possible, I think we should use MarkBind features on this page to present the appropriate information. Not suggesting that we go back to the current site where we include different versions of the panel state to showcase the style of the header when expanded or minimized, but use appropriate components to display information. Panels, box, tooltip, etc are all possible.

I'll look at the main page and other relevant sections once more, and see what we can add to the template without bloating it too much. Ideally, it should be long enough to make the page navigation a useful addition (currently it's too short for the page nav to be useful), while making use of a good number of MarkBind components, but not so much to the point of bloating it.

@lhw-1
Copy link
Contributor Author

lhw-1 commented Mar 26, 2023

I'll look at the main page and other relevant sections once more, and see what we can add to the template without bloating it too much. Ideally, it should be long enough to make the page navigation a useful addition (currently it's too short for the page nav to be useful), while making use of a good number of MarkBind components, but not so much to the point of bloating it.

I have updated the template to include more content (to make our pageNav more useful & to show off some of our components)! You can check out the new version at the deployed site here.

@lhw-1 lhw-1 mentioned this pull request Mar 26, 2023
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 @lhw-1, some nits. I think it's almost there.

packages/core/template/default/index.md Outdated Show resolved Hide resolved
packages/core/template/default/index.md Outdated Show resolved Hide resolved
packages/core/template/default/index.md Outdated Show resolved Hide resolved
packages/core/template/default/index.md Show resolved Hide resolved
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.

Some nits, otherwise LGTM for me

packages/core/template/default/index.md Outdated Show resolved Hide resolved
packages/core/template/default/index.md Outdated Show resolved Hide resolved
packages/core/template/default/index.md Outdated Show resolved Hide resolved
@tlylt tlylt requested a review from a team March 29, 2023 00:57
@tlylt tlylt removed the request for review from a team March 29, 2023 00:58
Copy link
Contributor

@yucheng11122017 yucheng11122017 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 the work @lhw-1! This seems to be a much better landing page than the current 👍
Some minor nits on my end!

packages/core/template/default/index.md Outdated Show resolved Hide resolved
packages/core/template/default/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jovyntls jovyntls left a comment

Choose a reason for hiding this comment

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

LGTM, really minor nits!

Some smaller thoughts:

  • "Placeholder Topic Pages" is a bit of a strange title for the page nav. I'm thinking "On this page" might be better?
  • The "What did you just do?" header sounds a bit negative to me, a bit like "What have you done?!?!" 😅 I think this is really minor so I'm fine with leaving it as it is. Other suggestions: "What's going on?", or no header at all

packages/core/template/default/index.md Outdated Show resolved Hide resolved
packages/core/template/default/index.md Outdated Show resolved Hide resolved
packages/core/template/default/index.md Show resolved Hide resolved
@lhw-1
Copy link
Contributor Author

lhw-1 commented Mar 29, 2023

Some smaller thoughts:

  • "Placeholder Topic Pages" is a bit of a strange title for the page nav. I'm thinking "On this page" might be better?

  • The "What did you just do?" header sounds a bit negative to me, a bit like "What have you done?!?!" sweat_smile I think this is really minor so I'm fine with leaving it as it is. Other suggestions: "What's going on?", or no header at all

For the first point, @tlylt pointed it out earlier, so I changed it to "Topics"!

As for the second point, that's a good point - I've changed it to "What just happened?" 😅

Copy link
Contributor

@jovyntls jovyntls left a comment

Choose a reason for hiding this comment

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

LGTM! 🎊

@tlylt tlylt requested a review from yucheng11122017 March 30, 2023 09:24
@yucheng11122017
Copy link
Contributor

Hi @lhw-1, I'm getting some errors when trying to use the page nav at the side. It doesn't move to the correct header which is really odd... Not sure if its something wrong with the deployment or is it a local thing as well?

@tlylt
Copy link
Contributor

tlylt commented Mar 30, 2023

Hi @lhw-1, I'm getting some errors when trying to use the page nav at the side. It doesn't move to the correct header which is really odd... Not sure if its something wrong with the deployment or is it a local thing as well?

@lhw-1 can you try building and deploy a version with the latest web built. It could be due to the recent updates to anchor scroll.

@lhw-1
Copy link
Contributor Author

lhw-1 commented Mar 30, 2023

Hi @lhw-1, I'm getting some errors when trying to use the page nav at the side. It doesn't move to the correct header which is really odd... Not sure if its something wrong with the deployment or is it a local thing as well?

@lhw-1 can you try building and deploy a version with the latest web built. It could be due to the recent updates to anchor scroll.

I've just tried deploying the latest site and locally serving it, and the page nav is not working on either. But I've also tried switching to the master branch to initialize a new site (with the previous version of default template) and the page nav is not working there either, so it seems like this issue is not local to this PR.

Edit: My web was not built to the latest 😅

@tlylt
Copy link
Contributor

tlylt commented Mar 30, 2023

Hi @lhw-1, I'm getting some errors when trying to use the page nav at the side. It doesn't move to the correct header which is really odd... Not sure if its something wrong with the deployment or is it a local thing as well?

@lhw-1 can you try building and deploy a version with the latest web built. It could be due to the recent updates to anchor scroll.

I've just tried deploying the latest site and locally serving it, and the page nav is not working on either. But I've also tried switching to the master branch to initialize a new site (with the previous version of default template) and the page nav is not working there either, so it seems like this issue is not local to this PR.

Did you markbind serve -d

@lhw-1
Copy link
Contributor Author

lhw-1 commented Mar 30, 2023

@lhw-1 can you try building and deploy a version with the latest web built. It could be due to the recent updates to anchor scroll.

Ok, I've built the latest web and re-deployed - the page nav for the deployment now seems to work 😅

Copy link
Contributor

@yucheng11122017 yucheng11122017 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot @lhw-1

@lhw-1 lhw-1 changed the title Update default template with UG links Update default template with UG links and useful information Apr 2, 2023
@lhw-1 lhw-1 added this to the v4.1.1 milestone Apr 2, 2023
@lhw-1 lhw-1 merged commit 67b37cb into MarkBind:master Apr 2, 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.

5 participants