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.

Sign up for GitHub

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

Jump to bottom

DOP-4594: Breadcrumbs and Internal Navigation buttons use TOC labels #1093

Merged
merged 16 commits into from
May 28, 2024
Merged

DOP-4594: Breadcrumbs and Internal Navigation buttons use TOC labels #1093

merged 16 commits into from
May 28, 2024

Conversation

mmeigs
Copy link
Collaborator

@mmeigs mmeigs commented May 9, 2024 •
edited
Loading

Stories/Links:

DOP-4594

Current Behavior:

Atlas

  • if you look at the bottom of the page for the "back button", the label now in prod is Getting Started with Atlas, rather than just Getting Started

Staging Links:

Atlas

  • You can now see that the breadcrumb is Getting Started and the back button at the bottom of the page is as well!
  • NOTE: the TOC is still incorrect lol I dropped a ticket in for that. You'll see that the TOC says "Getting Started with Atlas" but in prod it is "Getting Started".

Notes

This depends on a parser change which is merged yet not released!

README updates

    • This PR introduces changes that should be reflected in the README, and I have made those updates.
    • This PR does not introduce changes that should be reflected in the README

seungpark
seungpark reviewed May 21, 2024
View reviewed changes
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

minor point to not double title and plaintext if they are both strings

src/utils/setup/transform-breadcrumbs.js Outdated
return {
path,
title,
plaintext: getPlaintext(title),
plaintext: title,
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need both title and plaintext fields here? seems like we could do away with plaintext if we are using the same string for both

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're totally right. Done!

@mmeigs mmeigs requested a review from seungpark May 28, 2024 14:38
seungpark
seungpark approved these changes May 28, 2024
View reviewed changes
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

this LGTM 👍

@mmeigs mmeigs merged commit 4de916e into main May 28, 2024
2 checks passed
@mmeigs mmeigs deleted the DOP-4594-slug-breadcrumblabel branch May 28, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seungpark seungpark seungpark approved these changes

@i80and i80and Awaiting requested review from i80and

@schmalliso schmalliso Awaiting requested review from schmalliso

@anabellabuckvar anabellabuckvar Awaiting requested review from anabellabuckvar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mmeigs @seungpark