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 carets #1567

Merged
merged 5 commits into from
Jun 4, 2024
Merged

update carets #1567

merged 5 commits into from
Jun 4, 2024

Conversation

s-santillan
Copy link
Collaborator

@s-santillan s-santillan commented May 22, 2024

Thanks for improving Semgrep Docs 😀

Please ensure

  • You review the content
  • A technical writer reviews the content or PR

This PR...

  1. Makes the carets smaller, they seem huge.
  2. Adds carets to the top-level categories in the sidebar so that people know they are clickable sections. See the original request to understand the origins of this bug.

Why am I tagged for this random PR?

Because I value your opinion.

What do I do?

Check the Netlify deploy preview. Don't forget to append /docs at the end of it! We all have different 2nd monitors, browser default font sizes, laptop screens, etc, so a variety would be good.

@s-santillan s-santillan self-assigned this May 22, 2024
Copy link

netlify bot commented May 22, 2024

Deploy Preview for semgrep-docs-prod ready!

Name Link
🔨 Latest commit 6b344f4
🔍 Latest deploy log https://app.netlify.com/sites/semgrep-docs-prod/deploys/665f5cf798d3bd0008904ffb
😎 Deploy Preview https://deploy-preview-1567--semgrep-docs-prod.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

@khorne3 khorne3 left a comment

Choose a reason for hiding this comment

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

Much improved!

@pabloest
Copy link
Member

I don’t see the carets on mobile:

CleanShot 2024-05-22 at 18 55 36

Also I think we might want to rethink the phrase/label “Back to main menu” as I think it may be commonly understood to be about the main docs menu, not the main Semgrep menu. But that can be a separate thing.

@adamboros
Copy link
Contributor

I don’t see the carets on mobile:

This only happens on the root level for some reason, one level down the carets are there:
image

Also I think we might want to rethink the phrase/label “Back to main menu” as I think it may be commonly understood to be about the main docs menu, not the main Semgrep menu. But that can be a separate thing.

Maybe a simple 'Back' button would be enough.

The new caret size looks right to me! ✨

@s-santillan
Copy link
Collaborator Author

Fixed the missing carets. Almost there:

image

@s-santillan s-santillan merged commit d2cbbe2 into main Jun 4, 2024
7 checks passed
@s-santillan s-santillan deleted the sara/tec-42-add-carets-for-mobile-view branch June 4, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants