Skip to content

CB-12320: (iOS) WIP Add support for navigation bar #206

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

spongessuck
Copy link

Platforms affected

iOS

What does this PR do?

  • Adds support for using the iOS navigation bar as an alternative to a top-positioned toolbar. This makes the toolbar look much more native. When navigationbar=yes is set, it puts the toolbar buttons in the navigation bar and hides the toolbar.
  • Allows for the navigation buttons to be hidden using navigationbuttons=no.
  • Allows for adding a title to the navigation bar using navigationbartitle.

What testing has been done on this change?

Tested to make sure the new options are respected. I'm not sure what kind of automated tests might be required/have to be changed.

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

Issues

  • Location bar doesn't appear correctly when toolbarposition=top is set.

@cordova-qa
Copy link

Cordova CI Build has completed successfully.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 10 Store Link Link Link
iOS Link Link Link
Android 4.4 Link Link Link
Android 5.1 Link Link Link

@reidab
Copy link
Contributor

reidab commented Jan 17, 2017

Pulled down this PR and in my local testing with navigationbar=yes and navigationbuttons=yes the back/forward buttons appear to be reversed.

screen shot 2017-01-17 at 12 59 53 pm

@spongessuck
Copy link
Author

@reidab Thanks, fixed in af70ef3.

@cordova-qa
Copy link

Cordova CI Build has completed successfully.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 10 Store Link Link Link
iOS 9.3 Link Link Link
iOS 10.0 Link Link Link
Android 4.4 Link Link Link
Android 5.1 Link Link Link

@rcprdf
Copy link

rcprdf commented Feb 7, 2017

In my local testing of this PR with navigationbar=yes the status bar was never visible
or for that matter without navigationbar= when displaying a plain toolbar
the space reserved for it was empty

Seems to be a consequence of the tmpWindow / tmpController passed to presentViewController, code introduced by the prior PR CB-11136 ("InAppBrowser fails to close with WKWebView OAuth") - when I back out that fix the status bar is displayed properly above the navigation bar (or toolbar), and everything seems to work - at least with UIWebView, I have not tried it with WKWebView

@reidab
Copy link
Contributor

reidab commented Feb 8, 2017

@rcprdf I think the statusbar is being hidden behind the IAB window. I submitted PR #209 to fix this a few weeks back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants