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

adding fullscreenNavigationUI option; fix #482 #483

Merged

Conversation

cevaris
Copy link
Contributor

@cevaris cevaris commented Jun 11, 2021

Summary
On devices with soft UI buttons (Android/Amazon Fire, etc), when in fullscreen mode, some browsers still renders the soft UI buttons at the bottom of the screen, taking away from the "fullscreen" experience.

This PR adds a new Viewer option fullscreenNavigationUI, to modify that soft UI button display behavior per Mozilla web spec.

Fixes issue #482.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:
Before: before
After: after

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

  • Yes
  • No

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

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome/Brave
  • Firefox
  • Safari
  • Edge
  • IE
  • Silk

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

I realize the contributing guidelines ask for a reproducible example.
Although, this particular issues requires a particular device with soft UI buttons and is reproducible via master branch.
Hopefully, that explanation is sufficient and this PR will be allowed.
Thanks!

@cevaris cevaris changed the title adding fullscreenNavigationUI option; fix #482[,#482] adding fullscreenNavigationUI option; fix #482 Jun 11, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2021

Codecov Report

Merging #483 (cd313a9) into master (cdd94b4) will decrease coverage by 0.12%.
The diff coverage is 0.00%.

❗ Current head cd313a9 differs from pull request most recent head c066a63. Consider uploading reports for the commit c066a63 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
- Coverage   75.93%   75.80%   -0.13%     
==========================================
  Files           8        8              
  Lines        1421     1422       +1     
==========================================
- Hits         1079     1078       -1     
- Misses        342      344       +2     
Impacted Files Coverage Δ
src/js/others.js 51.42% <0.00%> (-1.00%) ⬇️
src/js/methods.js 88.64% <0.00%> (-0.03%) ⬇️
src/js/render.js 93.96% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdd94b4...c066a63. Read the comment docs.

@fengyuanchen
Copy link
Owner

fengyuanchen commented Jun 12, 2021

Please do not push any automatically generated files, such as any files in the dist folder.

@fengyuanchen
Copy link
Owner

Your idea is good, but your solution may be a bit one-sided.

@fengyuanchen fengyuanchen force-pushed the add-fullscreenNavigationUI-option branch 2 times, most recently from dac1cd4 to ae69808 Compare June 12, 2021 07:29
@fengyuanchen fengyuanchen force-pushed the add-fullscreenNavigationUI-option branch from ae69808 to 8214a36 Compare June 12, 2021 07:30
@fengyuanchen fengyuanchen merged commit 8fcb2be into fengyuanchen:master Jun 12, 2021
@cevaris cevaris deleted the add-fullscreenNavigationUI-option branch June 12, 2021 22:36
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.

3 participants