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

Fix: Theme preview in Mobile Chrome, Safari #1683

Merged
merged 2 commits into from
Dec 10, 2021

Conversation

iNuanfeng
Copy link
Contributor

Summary

I fix the sidebar style in mobile, when the body height is too small.

before after
IMG_5942 IMG_5943 2

What kind of change does this PR introduce?

Bugfix

For any code change,

  • Related documentation has been updated if needed
  • Related tests have been updated or tests have been added

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

Related issue, if any:

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

@vercel
Copy link

vercel bot commented Dec 6, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/BFAzBQc3zneuuviEnrAiqqYu6EEV
✅ Preview: https://docsify-preview-git-fork-inuanfeng-develop-docsify-core.vercel.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 6, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3399977:

Sandbox Source
docsify-template Configuration

@sy-records
Copy link
Member

Can you provide a reproducible environment or repo?

@Koooooo-7
Copy link
Member

Concurred with @sy-records .

@trusktr
Copy link
Member

trusktr commented Dec 6, 2021

@sy-records @Koooooo-7 the repo is linked at the top (https://github.com/iNuanfeng/docsify), and the vercel bot provided the live demo.

I tested it (deleted a bunch of elements in devtools inspector) but was not able to reproduce:

Capture

But I didn't try on an iPhone. Anyone else with an iPhone?

@sy-records
Copy link
Member

It's normal for me to use iPhone to view.

image

@Koooooo-7 Koooooo-7 added the wait for information something is not clear, waiting for the author of the issue/pr label Dec 6, 2021
@iNuanfeng
Copy link
Contributor Author

iNuanfeng commented Dec 7, 2021

@sy-records @Koooooo-7 the repo is linked at the top (https://github.com/iNuanfeng/docsify), and the vercel bot provided the live demo.

I tested it (deleted a bunch of elements in devtools inspector) but was not able to reproduce:

Capture

But I didn't try on an iPhone. Anyone else with an iPhone?

It will reappear on the iPhone.
My IOS version is 13.4.1
The chrome version on my iPhone is 87.0.4280.163

@iNuanfeng
Copy link
Contributor Author

iNuanfeng commented Dec 7, 2021

@sy-records When the content of the document is too small, it will be reproduced

@iNuanfeng
Copy link
Contributor Author

iNuanfeng commented Dec 7, 2021 via email

@sy-records
Copy link
Member

I don't have that version of the device.

I am using IOS15 and Chrome 97 is working fine.

https://github.com/iNuanfeng/iNuanfeng.github.io

@trusktr
Copy link
Member

trusktr commented Dec 7, 2021

@sy-records did you try with a small amount of content? Your screen shows a lot of content beyond the end of the bottom of the screen. See our screenshots that have little content.

@sy-records
Copy link
Member

Yes, I tested it with his repo.

@trusktr
Copy link
Member

trusktr commented Dec 7, 2021

@sy-records but in your screenshot (below), I see full content. Can you post a screenshot with small (not tall) content?

It's normal for me to use iPhone to view.

image

@sy-records
Copy link
Member

image

@iNuanfeng
Copy link
Contributor Author

@sy-records So, It may be a browser compatibility

@trusktr
Copy link
Member

trusktr commented Dec 8, 2021

Hmm, yeah, just can't reproduce it yet.

But, if this fixes your issue, I think it is safe to merge because our snapshot tests are passing.

Wdyt @Koooooo-7 @sy-records ?

@trusktr trusktr merged commit 47cd3b6 into docsifyjs:develop Dec 10, 2021
@iNuanfeng
Copy link
Contributor Author

@trusktr @sy-records @Koooooo-7 Thank you !

@trusktr
Copy link
Member

trusktr commented Dec 10, 2021

@iNuanfeng no prob!

@Koooooo-7 @sy-records we should add iPhone/Android tests. I learned how to configure playwright for that yesterday. WIP..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wait for information something is not clear, waiting for the author of the issue/pr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants