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(android): tabgroup badge number #11659

Merged
merged 3 commits into from Oct 8, 2020
Merged

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented Apr 26, 2020

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

  • badge >= 0 will show the badge
  • badge = null will remove it

Screenshot_20200426-175254

function createTab(title) {
	var window = Ti.UI.createWindow({ title: title , theme: "Theme.MyTheme"});
	window.add(Ti.UI.createLabel({ text: title + " View" }));
	var tab = Ti.UI.createTab({
		title: title,
		icon: "/assets/images/tab1.png",
		badgeColor: "#f00",
		window: window,
	});
	return tab;
}

var tabGroupSettings = {
	tabs: [createTab("Tab 1"), createTab("Tab 2"), createTab("Tab 3")],
};
if (OS_ANDROID) {
	tabGroupSettings.shiftMode = false;
	tabGroupSettings.style = Ti.UI.Android.TABS_STYLE_BOTTOM_NAVIGATION;
}
var tabGroup = Ti.UI.createTabGroup(tabGroupSettings);
tabGroup.addEventListener("open", function() {
	tabGroup.tabs[1].badge = "42";


	setTimeout(function(){
		tabGroup.tabs[1].badge = null;

		setTimeout(function(){
			tabGroup.tabs[1].badge = "23";
		}, 2000)
	}, 2000)
});
tabGroup.open();

@build
Copy link
Contributor

build commented Apr 26, 2020

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

💾 Here's the generated SDK zipfile.

📖 🎉 Another contribution from our awesome community member, m1ga! Thanks again for helping us make Titanium SDK better. 👍
📖 ✊ 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 984 skipped out of 10919 total tests.

Tests:

ClassnameNameTimeError
ios.ipad.Titanium.UI.iOS.CollisionBehavior.exampleworks (14.0)15.003
Error: timeout of 15000ms exceeded
file:///Users/build/Library/Developer/CoreSimulator/Devices/6C7D2BAE-178F-423C-B035-5B9A681D728F/data/Containers/Bundle/Application/D3460814-D13F-4EA1-B27E-E2AEC9A83F36/mocha.app/ti-mocha.js:4326:27
ios.macos.Titanium.UI.iOS.CollisionBehavior.exampleworks (10.15.4)15.138
Error: timeout of 15000ms exceeded
file:///Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-11659/tmp/mocha/build/iphone/build/Products/Debug-maccatalyst/mocha.app/Contents/Resources/ti-mocha.js:4326:27

Generated by 🚫 dangerJS against a35679d

@jquick-axway jquick-axway self-requested a review April 27, 2020 17:05
@m1ga
Copy link
Contributor Author

m1ga commented Apr 29, 2020

added the missing badgeColor

@sgtcoolguy
Copy link
Contributor

@m1ga Hope you don't mind - I've rebased this PR on master, fixed the merge conflicts, cleaned up the commits and added tests.

@m1ga
Copy link
Contributor Author

m1ga commented Aug 3, 2020

@sgtcoolguy 👍 no problem, glad it is not forgotten or closed 😄

@jquick-axway
Copy link
Contributor

@m1ga , PR #11921 changes Titanium default app theme to Theme.MaterialComponents.Bridge which maintains the exact same dark theme Theme.AppCompat uses (because it derives from it), but also provides the styles needed by Google's material widgets such as tab badges.

If we merge my PR first, then we don't need to change the unit test suite's app theme. We should still document that the material theme is required by this feature since many (or most?) Titanium developers tend to set up their own app theme which typically derives from an AppCompat theme.

@m1ga
Copy link
Contributor Author

m1ga commented Aug 18, 2020

@jquick-axway I already have On the Android platform you will need to use a Theme with parent="Theme.MaterialComponents.*" in order to use a badge. in the apidoc. Should be enough, or?

@jquick-axway
Copy link
Contributor

@m1ga, yes, your api doc changes are fine. Once my PR is merged, we can remove the following unit test files from this PR since my PR already makes MaterialComponents the default theme. (Sorry for not being clear.)

  • build/lib/test/test.js
  • tests/platform/android/res/values/styles.xml

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.

@m1ga , can you remove the following files from this PR please?
(Or we can do it if you're too busy.)

  • build/lib/test/test.js
  • tests/platform/android/res/values/styles.xml

PR #11921 makes Titanium use the material theme by default and it has now been merged. We want your unit tests to verify that tab badges work with the default theme.

@m1ga
Copy link
Contributor Author

m1ga commented Sep 28, 2020

@jquick-axway I've removed the two files but it is still complaining about a merge conflict with build/lib/test/test.js (and the others).

edit: found the solution 😄

@m1ga
Copy link
Contributor Author

m1ga commented Sep 29, 2020

Added the changes and tested it with your test code. Added a timeout to reset and set it again. Updated the code in the first post

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

@jquick-axway
Copy link
Contributor

Thanks @m1ga . Your final changes look great!

@ssjsamir ssjsamir self-requested a review October 8, 2020 13:32
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, Tested using the test case mentioned in the description.

Test Environment

MacOS Big Sur: 11.0 Beta 8
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.0)

@sgtcoolguy sgtcoolguy merged commit e1f6909 into tidev:master Oct 8, 2020
@m1ga m1ga deleted the showBadge branch March 3, 2021 21:36
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