Skip to content

Update headings to match sidebar#2152

Merged
jovyntls merged 8 commits intoMarkBind:masterfrom
itsyme:tweak-headings
Feb 17, 2023
Merged

Update headings to match sidebar#2152
jovyntls merged 8 commits intoMarkBind:masterfrom
itsyme:tweak-headings

Conversation

@itsyme
Copy link
Contributor

@itsyme itsyme commented Feb 9, 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 #2150. All headers which did not match the title in the sidebar were changed for both the User Guide and Developer Guide.

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Update headings to match sidebar


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

@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 @itsyme!

For user guide, the .gitignore file and glossary doesn't match as well.
For the dev guide, the introduction (under onboarding bootcamp) doesn't match.
DG the subpages under the github actions are titled "GitHub Actions: {{title}}". Same for subpage in Appendices. Not sure if this should be changed?

<include src="advanced.md#slots-info" />

# Pop-Up Components
# Using Components: Pop-ups
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nit: Should be Pop-Ups according to side bar

@itsyme
Copy link
Contributor Author

itsyme commented Feb 9, 2023

Thanks for the detailed review @yucheng11122017 !! I have made the relevant changes in the latest commit. For Onboarding Bootcamp -> Introduction, . As for "GitHub Actions: {{ title }}", the correct titles were set at the top of the page such that {{ title }} could be reused in other places in the page as well. I agree that it might be good to standardise the use of {{ title }} across all pages in the future.

@yucheng11122017
Copy link
Contributor

Thanks for the detailed review @yucheng11122017 !! I have made the relevant changes in the latest commit. For Onboarding Bootcamp -> Introduction, . As for "GitHub Actions: {{ title }}", the correct titles were set at the top of the page such that {{ title }} could be reused in other places in the page as well. I agree that it might be good to standardise the use of {{ title }} across all pages in the future.

Yup agree! It seems like only sub headings (like under components) have the {{ heading }}: {{ title }} format while the rest all use {{ title }}

@tlylt tlylt requested a review from a team February 12, 2023 02:27
@yucheng11122017
Copy link
Contributor

Thanks for the detailed review @yucheng11122017 !! I have made the relevant changes in the latest commit. For Onboarding Bootcamp -> Introduction, . As for "GitHub Actions: {{ title }}", the correct titles were set at the top of the page such that {{ title }} could be reused in other places in the page as well. I agree that it might be good to standardise the use of {{ title }} across all pages in the future.

Yup agree! It seems like only sub headings (like under components) have the {{ heading }}: {{ title }} format while the rest all use {{ title }}

Maybe this would be changed to {{ title }} to follow the rest

@itsyme
Copy link
Contributor Author

itsyme commented Feb 12, 2023

Hi @yucheng11122017! I think that's a great suggestion! However, I think this may be out of scope for this PR. Perhaps it could be good to open a new issue/PR to address this issue!

@jovyntls
Copy link
Contributor

What do you think of not having the Using Components: prefix on the component titles? I left a comment on the original issue about this - personally I'd prefer to not have the prefix since it's a bit verbose and this naming pattern is more suited for breadcrumbs than for a page title 😅

@yucheng11122017
Copy link
Contributor

yucheng11122017 commented Feb 13, 2023

What do you think of not having the Using Components: prefix on the component titles? I left a comment on the original issue about this - personally I'd prefer to not have the prefix since it's a bit verbose and this naming pattern is more suited for breadcrumbs than for a page title 😅

Since Prof is ok with it, @itsyme maybe you can update?
Yeah I agree. Especially if there are sub sub sub.. headings it'll make the title really unnecessary and long

@itsyme
Copy link
Contributor Author

itsyme commented Feb 14, 2023

Alright! I think that is cleaner as well. I will update it in the next commit!

@itsyme
Copy link
Contributor Author

itsyme commented Feb 14, 2023

Hi @yucheng11122017! I realised I have not resolved your comment about developer guide -> onboarding bootcamp -> introduction. I left the title as "Onboarding Bootcamp" as I felt that putting "Introduction" alone for the title might be a little too vague. What do you think?

@yucheng11122017
Copy link
Contributor

Hi @yucheng11122017! I realised I have not resolved your comment about developer guide -> onboarding bootcamp -> introduction. I left the title as "Onboarding Bootcamp" as I felt that putting "Introduction" alone for the title might be a little too vague. What do you think?

Yeah good point! Concur with you on that then

@jovyntls
Copy link
Contributor

jovyntls commented Feb 15, 2023

One last nit since we're standardising titles, can we make the capitalisation here consistent? ('sites'->'Sites')
image

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

@jovyntls jovyntls changed the title Tweak headings to match sidebar in documentation Update headings to match sidebar Feb 17, 2023
@jovyntls jovyntls merged commit c756c2c into MarkBind:master Feb 17, 2023
@jovyntls jovyntls added this to the v4.1.1 milestone Feb 17, 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.

User Guide: Tweak heading to match site navigation menu

3 participants