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

refactor(ios): change Ti.UI.TabbedBar to use UITabBar #12511

Closed
wants to merge 1 commit into from

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Mar 2, 2021

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

Summary:

  • This changes Ti.UI.TabbedBar to use a tab bar instead of a segmented control.
  • Titanium 10.0.0's new Ti.UI.OptionBar is now used to display a segmented control on iOS. See: TIMOB-28317
  • Intention is that a widget named TabbedBar should show tabs, like it does on Android, and allow app devs to layout their own tab group like user interface.

TabbedBar-iOS

Test:

  1. Build and run TabbedBarTest.js attached to TIMOB-28373 on iOS.
  2. Verify that the app shows tabs as shown in the screenshot above.
  3. Tap on the 2nd tab of every bar and verify the following gets logged.
    @@@ button click index: 1

@jquick-axway jquick-axway added ios docs BREAKING CHANGES ⚠️ parity 👯‍♀️ backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge labels Mar 2, 2021
@jquick-axway jquick-axway added this to the 10.0.0 milestone Mar 2, 2021
@build build requested review from a team March 2, 2021 06:55
@build
Copy link
Contributor

build commented Mar 2, 2021

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

Test suite crashed on iOS simulator. Please see the crash log for more details.

🚫

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.

Warnings
⚠️ This PR has milestone set to 10.0.0, but the version defined in package.json is 10.1.0 Please either: - Update the milestone on the PR - Update the version in package.json - Hold the PR to be merged later after a release and version bump on this branch
Messages
📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖 ❌ 2 tests have failed There are 2 tests failing and 682 skipped out of 11676 total tests.

Tests:

ClassnameNameTimeError
android.emulator.Titanium.UI.View"after all" hook (5.0.2)20.542
Error: timeout of 10000ms exceeded
at Titanium.<anonymous> (/ti-mocha.js:6535:53723)
android.emulator.Titanium.UI.View"after each" hook (5.0.2)10.533
Error: timeout of 10000ms exceeded
at Titanium.<anonymous> (/ti-mocha.js:6535:53723)

Generated by 🚫 dangerJS against 039318e

@hansemannn
Copy link
Collaborator

Please reconsider if this is really a good idea. The UITabBar is a top-level element on iOS and should not be added to a view directly. It's probably even causing App Store rejections and can be easily overseen by developers that have larger apps and upgrade to SDK 10 (which, in terms of the recent news) will not really help developers.

@vijaysingh-axway
Copy link
Contributor

  1. As per Apple doc here, UITabBar is typically used in conjunction with UITabBarController. In titanium Ti.UI.TabGroup is used to implement UITabBarController.
  2. I don't think adding UITabBar as standalone component will cause Appstore rejection.
  3. Do we want to make breaking change for this, which people are using from long and I think no one is asking for this?

@hansemannn
Copy link
Collaborator

hansemannn commented Mar 3, 2021

@vijaysingh-axway Regarding (2), see this link for reference, especially:

Use a tab bar strictly for navigation. Don’t use tab bar buttons to enable actions. If you need to provide controls that act on elements in the current view, use a toolbar instead.

As well as the fact that button-only tabbed bars (which is the only default right now) will cause major display issues, because UITab's are meant to have an icon and without it, the title will be displayed at the bottom (see screenshot above).

@sgtcoolguy sgtcoolguy removed the backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge label Mar 11, 2021
@jquick-axway
Copy link
Contributor Author

No one here is suggesting that UITabBar should be used as a toolbar or as a segmented control replacement. The main point here is that Ti.UI.TabbedBar is misnamed for the native component currently used on iOS. The solution is to create a new Ti.UI.OptionBar to show a segmented control (already merged into Titanium 10.0.0) and change Ti.UI.TabbedBar to use UITabBar so app developers can implement their own custom tab group like interface.

If you feel this is controversial, then I can close this PR for 10.0.0. This means iOS will use a native UISegmentedControl for both Ti.UI.TabbedBar and Ti.UI.OptionsBar.

On Android, Ti.UI.TabbedBar uses the native TabLayout and BottomNavigationView. The new Ti.UI.OptionBar uses the native MaterialToggleButtonGroup. We're not changing the Android side. The native widgets being used on Android make sense.

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