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(android): remove getter and setter methods from TabGroup #12661

Merged
merged 3 commits into from Apr 5, 2021

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Mar 25, 2021

  • Remove getActiveTab() and setActiveTab() from Ti.UI.TabGroup
  • Remove getTabs() from Ti.UI.TabGroup
  • Remove overiding of TabGroup via common extension
TEST CASE
const win_a = Ti.UI.createWindow({
	backgroundColor: 'blue',
	title: 'Blue'
});
const win_b = Ti.UI.createWindow({
	backgroundColor: 'red',
	title: 'Red'
});
const tab_a = Ti.UI.createTab({
	window: win_a,
	title: 'Blue'
});
const tab_b = Ti.UI.createTab({
	window: win_b,
	title: 'Red'
});
const tabGroup = Ti.UI.createTabGroup({
	tabs: [ tab1, tab2 ],
	activeTab: 1 // Open on second tab.
});

win_a.addEventListener('click', e => {

	// Switch to second tab.
	tabGroup.activeTab = 1;
});

win_b.addEventListener('click', e => {

	// Reset tabs.
	tabGroup.tabs = [ tab_a, tab_b ];
});

tabGroup.open();

JIRA Ticket

@garymathews garymathews added android bug no tests 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 25, 2021
@build build added this to the 10.1.0 milestone Mar 25, 2021
@build build requested review from a team March 25, 2021 16:56
@build build added the docs label Mar 25, 2021
@build
Copy link
Contributor

build commented Mar 25, 2021

Fails
🚫 Tests have failed, see below for more information.
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 501 skipped out of 7017 total tests.
📖 👍 Hey!, You deleted more code than you added. That's awesome!

Tests:

ClassnameNameTimeError
ios.macos.fs#truncate() truncates to specified number of bytes (10.15.5)6.014
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
ios.macos.Titanium.Filesystem.File.remoteBackup assigning Boolean value doesn't throw (10.15.5)11.001
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
run@file:///ti.main.js:8744:22
processImmediateQueue@file:///ti.main.js:8807:18
drainQueues@file:///ti.main.js:8784:52

Generated by 🚫 dangerJS against df8850e

@garymathews garymathews force-pushed the TIMOB-28404 branch 2 times, most recently from f47e428 to 375548d Compare March 25, 2021 18:06
Copy link
Contributor

@jquick-axway jquick-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: Pass

@@ -263,11 +263,6 @@ methods:
summary: Tab to remove.
type: Titanium.UI.Tab

- name: getTabs
Copy link
Contributor

Choose a reason for hiding this comment

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

I already pushed the change to master and 10_0_X (see 580cffc), but we don't want to just remove API from our apidocs. We need to explicitly mark it as deprecated and/or removed and retain it in the docs until the removed version has gone EOS/EOL. Otherwise it won't be documented/exist anymore and we want devs on older SDK versions to still see the APIs that are deprecated/removed in current/future versions.

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, using the following test case:

const win_a = Ti.UI.createWindow({
	backgroundColor: 'blue',
	title: 'Blue'
});
const win_b = Ti.UI.createWindow({
	backgroundColor: 'red',
	title: 'Red'
});
const tab_a = Ti.UI.createTab({
	window: win_a,
	title: 'Blue'
});
const tab_b = Ti.UI.createTab({
	window: win_b,
	title: 'Red'
});
const tabGroup = Ti.UI.createTabGroup({
	tabs: [ tab_a, tab_b ],
	activeTab: 1 // Open on second tab.
});

win_a.addEventListener('click', e => {

	// Switch to second tab.
	tabGroup.activeTab = 1;
});

win_b.addEventListener('click', e => {

	// Reset tabs.
	tabGroup.tabs = [ tab_a, tab_b ];
});

tabGroup.open();

Test Environment

MacOS Big Sur: 11.1 Beta 1
Xcode: 12.2 Beta
Java Version: 1.8.0_242
Android NDK: 21.3.6528147
Node.js: 12.18.1
""NPM":"5.0.0","CLI":"8.1.1""
Pixel XL (10) emulator

@sgtcoolguy sgtcoolguy merged commit 925e1cd into tidev:master Apr 5, 2021
@build
Copy link
Contributor

build commented Apr 5, 2021

The backport to 10_0_X failed:

The process 'git' failed with exit code 128

Check the run for full details
To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Check out the target branch
git checkout 10_0_X
# Make sure it's up to date
git pull
# Check out your branch
git checkout -b backport-12661-to-10_0_X
# Apply the commits from the PR
curl -s https://github.com/appcelerator/titanium_mobile/commit/caec617cb0eb76f9b741af9c01793da0b5785dbc.patch | git am -3 --ignore-whitespace
# Push it to GitHub
git push --set-upstream origin backport-12661-to-10_0_X

Then, create a pull request where the base branch is 10_0_X and the compare/head branch is backport-12661-to-10_0_X.

sgtcoolguy pushed a commit that referenced this pull request Apr 5, 2021
@ewanharris ewanharris removed 10_0_X backport failed backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge labels May 4, 2021
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