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

Reduce the number of admonitions across the site #1092

Merged
merged 1 commit into from
Apr 24, 2022
Merged

Conversation

jonaharagon
Copy link
Member

We don't want to over-use them, because they should specifically call out important information. We have a number of (especially "notes" and "info") admonitions that could just as easily be plain-text, because their content isn't particularly more important than other content on the page.

@netlify
Copy link

netlify bot commented Apr 23, 2022

Deploy Preview for privacyguides ready!

Name Link
🔨 Latest commit 3136961
🔍 Latest deploy log https://app.netlify.com/sites/privacyguides/deploys/626563d28292dc00084340f3
😎 Deploy Preview https://deploy-preview-1092--privacyguides.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 settings.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

What's the rationale behind moving some of the call-outs above the recommendation card? (e.g. Apple Mail and Tahoe LFS)

@jonaharagon
Copy link
Member Author

jonaharagon commented Apr 23, 2022

Basically I have it so that notes about choosing whether to use the recommendation are above the card (i.e. you don't probably want to use Apple Mail on iOS, you probably don't want to use Tahoe-LAFS unless you know what you're doing), and notes about actually using the recommendation are below the card (i.e. a note about JS E2EE, or a warning about potential IP leaks).

I don't know if that makes sense, does it? Maybe it just does to me :)

@ghost
Copy link

ghost commented Apr 23, 2022

Hm, yeah that makes sense. I don't really like how it pushes the card away from the header, but I can't think of a better way it could be done. Good change.

@dngray dngray added the t:correction content corrections or errors label Apr 24, 2022
dngray pushed a commit that referenced this pull request Apr 24, 2022
Signed-off-by: Daniel Gray <dng@disroot.org>
Signed-off-by: Daniel Gray <dng@disroot.org>
@dngray dngray merged commit 3136961 into main Apr 24, 2022
@dngray dngray deleted the pr-remove_admonitions branch April 24, 2022 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:correction content corrections or errors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants