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

feat(ios) : Added functionality to show search bar in navigation bar for TiUIListView and TiUITableView #10664

Merged
merged 10 commits into from Mar 7, 2019

Conversation

vijaysingh-axway
Copy link
Contributor

@vijaysingh-axway vijaysingh-axway commented Feb 1, 2019

@build
Copy link
Contributor

build commented Feb 1, 2019

Messages
📖

💾 Here's the generated SDK zipfile.

📖

✅ All tests are passing
Nice one! All 2995 tests are passing.

Generated by 🚫 dangerJS against 1083086

controller = [[TiApp app] controller];
}
if (!controller.navigationItem.searchController) {
controller.navigationItem.searchController = searchController;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the iOS 11 guard is required here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already taken care while assigning value to variable 'isSearchBarInNavigation'.
isSearchBarInNavigation = [TiUtils boolValue:[(TiViewProxy *)self.proxy valueForUndefinedKey:@"showSearchBarInNavigation"] def:NO] && [TiUtils isIOSVersionOrGreater:@"11.0"];

Copy link
Contributor

Choose a reason for hiding this comment

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

@vijaysingh-axway Since we still support older Xcode and SDK versions that don't have the searchController property we need the iOS 11 safeguard anyway. Probably something similar to #10729

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@janvennemann Wrapped under macro IS_XCODE_9.

default: false
since: 8.1.0
platforms: [iphone, ipad]
availability: creation
Copy link
Collaborator

Choose a reason for hiding this comment

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

osver: {ios: {min: "11.0"}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Thanks!

summary: A Boolean indicating whether search bar will be in navigation bar.
description: |
If you want to show the search bar in navigation bar, set this property `true` during creation.
Use the <Ti.UI.Window.hidesSearchBarWhenScrolling> property to control the visibility of the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do the docs allow to use Ti.UI instead of Titanium.UI these days?

@hansemannn
Copy link
Collaborator

I assume this does not work without ListView / TableView right now? Not too critical, but maybe required in the future.

@vijaysingh-axway vijaysingh-axway changed the title feat(ios) : Added functionality to show search bar in navigation bar for TiUIListView feat(ios) : Added functionality to show search bar in navigation bar for TiUIListView and TiUITableView Feb 4, 2019
@@ -1350,6 +1350,19 @@ properties:
platforms: [iphone, ipad]
availability: creation

- name: showSearchBarInNavigation
Copy link
Collaborator

Choose a reason for hiding this comment

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

Really quick and minor comment: Usually, the UINavigationBar is referenced as the navBar in Titanium, e.g. navBarHidde and hideNavBar / showNavBar. For the sake of parity, this could become showSearchBarInNavBar if you agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. Updated PR.

Copy link
Contributor

@janvennemann janvennemann left a comment

Choose a reason for hiding this comment

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

Added a minor note about required backwards compatibility with older Xcode/iOS SDK versions.

Copy link
Contributor

@janvennemann janvennemann left a comment

Choose a reason for hiding this comment

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

CR passed!

Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed. Able to see search bar in navigation bar for TiUIListView and TiUITableView. Tested with the following test cases:

var rows = [];

for (var i = 0; i < 20; i++) {
var title = i ? 'Row '+ i : 'Close Window'
rows.push({ properties: { title:title  , backgroundColor: 'red', searchableText:title}});
}
 
var win = Ti.UI.createWindow({
title: 'TEST',
backgroundColor: 'white',
});
 
var nav = Ti.UI.iOS.createNavigationWindow({
window: win,
});
 
var sb = Ti.UI.createSearchBar();
 
var ls = Ti.UI.createListSection({
items: rows
});
 
var lv = Ti.UI.createListView({
dimBackgroundForSearch: true,
sections: [ls],
searchView: sb,
showSearchBarInNavBar: true
});
 
sb.setHintText("test");
sb.addEventListener('change', function(e){
Ti.API.info(e.value);
});
sb.addEventListener('return', function(e){
sb.blur();
});
win.add(lv);
nav.open();

TiUIListView:

var rows = [];

for (var i = 0; i < 20; i++) {
var title = i ? 'Row '+ i : 'Close Window'
rows.push({ properties: { title:title  , backgroundColor: 'red', searchableText:title}});
}
 
var win = Ti.UI.createWindow({
title: 'TEST',
backgroundColor: 'white',
hidesSearchBarWhenScrolling: false
});
 
var nav = Ti.UI.createNavigationWindow({
window: win,
});
 
var sb = Ti.UI.createSearchBar({
hintTextColor: 'white'
});
 
var ls = Ti.UI.createListSection({
items: rows
});
 
var lv = Ti.UI.createListView({
dimBackgroundForSearch: true,
sections: [ls],
searchView: sb,
showSearchBarInNavBar: true
});
 
sb.setHintText("test");
sb.addEventListener('change', function(e){
Ti.API.info(e.value);
});
 
sb.addEventListener('return', function(e){
sb.blur();
});
 
win.add(lv);
nav.open();

TiUITableView:

var rows = [];

for (var i = 0; i < 10; i++) {
var title = i ? 'Row '+ i : 'Close Window';
rows.push({ title: title});
}
 
var win = Ti.UI.createWindow({
backgroundColor: 'red',
hidesSearchBarWhenScrolling: true
});
 
var sb = Ti.UI.createSearchBar({
hintTextColor: 'blue'
});
var lv = Ti.UI.createTableView({
data: rows,
search: sb,
showSearchBarInNavBar: true,
});
 
sb.setHintText("test");
sb.addEventListener('change', function(e){
Ti.API.info(e.value);
});
 
sb.addEventListener('return', function(e){
sb.blur();
});
 
sb.addEventListener('cancel', function(e){
});
lv.addEventListener('click', function(e) {
})
 
win.add(lv);
var nav = Ti.UI.iOS.createNavigationWindow({
window: win,
});
nav.open();

Test Environment

iPhone 6S plus (12.1 Sim)
APPC CLI: 7.0.10-17
Operating System Name: Mac OS Mojave
Operating System Version: 10.14.2
Node.js Version: 8.9.1
Xcode 10.1

@ssjsamir ssjsamir merged commit 99be2e2 into tidev:master Mar 7, 2019
@hansemannn
Copy link
Collaborator

Great to see this merged, thanks everyone!

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

5 participants