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

[TIMOB-25003] iOS: Deprecate Ti.UI.iOS.Toolbar in favor of cross-platform Ti.UI.Toolbar #9292

Merged
merged 6 commits into from Aug 17, 2017

Conversation

hansemannn
Copy link
Collaborator

JIRA: https://jira.appcelerator.org/browse/TIMOB-25003

Also clang-formatted the new source and made the deprecated API reuse the new one by subclassing it. Test-cases in JIRA:

* Licensed under the terms of the Apache Public License
* Please see the LICENSE included with this distribution for details.
*/
#ifdef USE_TI_UIIOSTOOLBAR
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be -
+#ifdef USE_TI_UITOOLBAR

@@ -1,24 +1,25 @@
/**
* Appcelerator Titanium Mobile
* Copyright (c) 2009-2010 by Appcelerator, Inc. All Rights Reserved.
* Copyright (c) 2009-2017 by Appcelerator, Inc. All Rights Reserved.
* Licensed under the terms of the Apache Public License
* Please see the LICENSE included with this distribution for details.
*/

#ifdef USE_TI_UIIOSTOOLBAR
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be -
+#ifdef USE_TI_UITOOLBAR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, we should use #if defined(USE_TI_UIIOSTOOLBAR) || defined(USE_TI_UITOOLBAR) all around the place, since Ti.UI.iOS.Toolbar will invoke TiUIToolbarProxy internally as well.


- name: borderTop
summary: If `true`, a border is drawn on the top of the toolbar.
summary: If `true`, a border is drawn on the top of the toolbar. This property is ignored on iOS 7 and above.
Copy link
Contributor

Choose a reason for hiding this comment

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

As this property do not have effect in iOS 7 and above and we are supporting iOS 8 and above, deprecate it .

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed all border-related docs, since the property does not even exist in the core anymore. Good catch, keeps it clean!

- name: borderBottom
summary: If `true`, a border is drawn on the bottom of the toolbar.
summary: If `true`, a border is drawn on the bottom of the toolbar. This property is ignored on iOS 7 and above.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@hansemannn
Copy link
Collaborator Author

@vijaysingh-axway Updated PR!

Copy link
Contributor

@vijaysingh-axway vijaysingh-axway 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.

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

3 participants