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(ios): set navigation controller’s background color #11184

Merged
merged 8 commits into from Sep 5, 2019

Conversation

vijaysingh-axway
Copy link
Contributor

@build
Copy link
Contributor

build commented Aug 28, 2019

Fails
🚫 Tests have failed, see below for more information.
Warnings
⚠️

Commit ba97fdb68526d62850272b1369b6f801c6a9b7a2 has a message "fix(ios): Supported barColor in iOS 13" giving 1 errors:

  • subject must not be sentence-case, start-case, pascal-case, upper-case
Messages
📖

💾 Here's the generated SDK zipfile.

📖 ❌ 1 tests have failed There are 1 tests failing and 472 skipped out of 4380 total tests.
📖

🚨 This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

Tests:

ClassnameNameTimeError
ios.os#hostname() returns Ti.Platform.address value0.001
Error: expected '' to equal undefined
fail@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/node_modules/should/cjs/should.js:275:23
value@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/node_modules/should/cjs/should.js:356:23
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/os.test.js:123:31
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/ti-mocha.js:4376:41
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/ti-mocha.js:4763:17
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/ti-mocha.js:4840:23
next@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/ti-mocha.js:4688:20
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/ti-mocha.js:4698:15
next@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/ti-mocha.js:4636:30
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/ti-mocha.js:4665:13
timeslice@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F8ABD09A-29F3-4078-9FA5-C369505A181F/mocha.app/ti-mocha.js:5764:29

Generated by 🚫 dangerJS against cce212c

@@ -220,6 +220,7 @@ - (void)navigationController:(UINavigationController *)navigationController will
[theWindow windowWillOpen];
[theWindow windowDidOpen];
}
navController.view.backgroundColor = theWindow.view.backgroundColor;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It feels like a workaround, since thats not necessary natively. Can you please go through the view tree and check the differences between native views / VC's?

@lokeshchdhry
Copy link
Contributor

FR Passed for this part of the fix.
Likely more fixes will be added for this issue in additional PR's.

Studio Ver: 5.1.3.201907112159
SDK Ver: 8.2.0.v20190903100931
OS Ver: 10.14.5
Xcode Ver: Xcode 11.0
Appc NPM: 4.2.15-1
Appc CLI: 7.1.1-master.6
Daemon Ver: 1.1.3
Ti CLI Ver: 5.2.1
Alloy Ver: 1.14.1
Node Ver: 10.16.1
NPM Ver: 6.9.0
Java Ver: 10.0.2
Simulator: Iphone XR ios 13, iphone XR ios 12.4

controller.navigationController.navigationBar.backgroundColor = UIColor.clearColor;
}
}
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks very very hacky and I feel like it's not fixing the actual issue but only workarounding the visual glitches. I hope there can be found a proper fix for this issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In addition, the navigation bar now shows a different color than the background color of the window.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hansemannn Yeah. It's kind of hack. In native apps as well people suggest same. See here. And this will work without any visual issue.

In context of property 'largeTitleEnabled' is 'true' in iOS 13-
A). As for as navigation bar color and transition similar to native behaviour is concerned. For that above SDK changes are not required. To make it work by default we have to make changes as per point C. If you want this to work in your app now, you have to modify your code-

  1. You have to set window's property 'extendEdges' to [Ti.UI.EXTEND_EDGE_TOP]. By default it is Ti.UI.EXTEND_EDGE_NONE.
  2. Set 'translucent' property to 'true'. By default it is 'true'
  3. If you are using tableview, listview or scrollview (basically view’s having scroll view) on window, it will work perfect.
  4. If you are using any other view other than views in point 3, you have to set ‘top’ property of view to height of navigation bar. Otherwise it will move up and overlap with navigation bar.

B). As you have mentioned navigation bar color is different than the bar color. I guess you are using "barColor" property to set. In SDK code barColor has preference over backgroundColor of window. See the attached examples in ticket, it is working fine. You always have option to set 'barColor'.

C). In native development, property 'edgesForExtendedLayout' is by default UIRectEdgeAll and default value of 'translucent' is 'true'. So it takes the background color of controller's view. If you use tableview or scrollview it will work perfect. If you want to create any other view with position (0, 0, width, height). It'll overlap with navigation bar. To avoid this problem there is option to use autolayout constraints.
In titanium auto layout is in beta. So we can not use that.
I'll create a new ticket to change default behaviour of window's property extendEdges to Ti.UI.EXTEND_EDGE_ALL, which apple recommends. And probably need to handle window's layout if there is navigation bar(which will need more work).

@sgtcoolguy sgtcoolguy merged commit 8049cbf into tidev:master Sep 5, 2019
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.

None yet

6 participants