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

DOP-5052: fixing link buffer's on mobile #1289

Merged
merged 10 commits into from
Oct 23, 2024
Merged

DOP-5052: fixing link buffer's on mobile #1289

merged 10 commits into from
Oct 23, 2024

Conversation

biancalaube
Copy link
Collaborator

@biancalaube biancalaube commented Oct 21, 2024

Stories/Links:

DOP-5052

Current Behavior:

  1. on the $merge page, the links in the field columns in the syntax table
  2. replSetGetStatus page, some lines in the output section have this behavior as well (such as electionCandidateMetrics )

Staging Links:

merge page

replSetGetStatus page

definition list item staging

Notes:

  • I got rid of the buffer variable in Permalink since it was never used and always undefined when I was testing. No other file called Permalink and gave a buffer value.
  • I tested locally with banner-dev and it seemed to work without changing any of the values, but feel free to check yourself
  • Make sure the links work on mobile, tablet and full screen
  • Check to make sure it didnt mess up other links

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

Copy link

netlify bot commented Oct 21, 2024

Deploy Preview for mongodb-snooty ready!

Name Link
🔨 Latest commit b7d7a9d
🔍 Latest deploy log https://app.netlify.com/sites/mongodb-snooty/deploys/6717f332676ad10008baa124
😎 Deploy Preview https://deploy-preview-1289--mongodb-snooty.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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.

see comments below!

src/components/Target.js Outdated Show resolved Hide resolved
src/components/Permalink.js Outdated Show resolved Hide resolved
src/theme/docsTheme.js Outdated Show resolved Hide resolved
src/components/Target.js Outdated Show resolved Hide resolved
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 points: 1) can we update the docstheme so that navbar has more buffer space (dont need to add another, but we can increase the existing)

and 2) update the staging link pls! it is still on an old commit

otherwise LGTM !

@biancalaube
Copy link
Collaborator Author

biancalaube commented Oct 22, 2024

@seungpark , changed the offset to be 95px but let me know if you want something different, staging links should work now!

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!

@biancalaube biancalaube merged commit eab275c into main Oct 23, 2024
6 checks passed
@biancalaube biancalaube deleted the DOP-5052 branch October 23, 2024 14:13
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.

2 participants