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

chore(android): material theme/widget improvements #12537

Merged
merged 8 commits into from Mar 16, 2021

Conversation

jquick-axway
Copy link
Contributor

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

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

Summary:

  • Changed Titanium default theme for "statusBarColor" and "navigationBarColor" to use "?attr/colorPrimaryDark".
    • Makes it easier to custom theme an app with just "colorPrimary" and "colorPrimaryDark".
    • Was defaulting to this color before, but in a way that wasn't as easy to override.
  • Added null reference guards to proxy classes in case it's used after release.
  • Updated TabGroup and TabbedBar top tab style to scale icons to fit like how bottom style works.
  • Improved Ti.UI.Button background and touch feedback handling.
    • Setting a "backgroundImage", "backgroundGradient", or "border*" properties used to log a nasty warning from the new MaterialButton we're using since it can cause it to be rendered incorrectly. Will now fallback to using AppCompatButton if you override its background.
    • Will now apply "touchFeedbackColor" correctly on MaterialButton when not overriding its background.

Test:

  1. Build and run the below code on Android.
  2. Verify that you do NOT see the following warning in the log.
    [WARN] MaterialButton: MaterialButton manages its own background to control elevation, shape, color and states...
  3. Verify that you see an orange button and a blue gradient button like in the screenshot below. (Note that the bottom blue image button currently fails to render on "master". This PR fixes it.)
  4. Tap on both buttons and verify that you see a yellow ripple effect.

AndroidButtons

const window = Ti.UI.createWindow();
const colorButton = Ti.UI.createButton({
	title: "Orange Button",
	color: "white",
	backgroundColor: "orange",
	touchFeedbackColor: "yellow",
	touchFeedback: true,
	top: "20%",
});
colorButton.addEventListener("click", () => {
	Ti.API.info("@@@ 'Orange Button' was clicked on.");
});
window.add(colorButton);
const imageButton = Ti.UI.createButton({
	title: "Image Button",
	color: "white",
	backgroundImage: "android.resource://android/drawable/panel_picture_frame_bg_focus_blue",
	touchFeedbackColor: "yellow",
	touchFeedback: true,
	bottom: "20%",
});
imageButton.addEventListener("click", () => {
	Ti.API.info("@@@ Image Button was clicked on.");
});
window.add(imageButton);
window.open();

@jquick-axway jquick-axway added android improvement 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 10, 2021
@jquick-axway jquick-axway added this to the 10.1.0 milestone Mar 10, 2021
@build build requested a review from a team March 10, 2021 07:18
@build
Copy link
Contributor

build commented Mar 10, 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 738 skipped out of 12500 total tests.

Tests:

ClassnameNameTimeError
android.emulator.Titanium.UI.View"after all" hook for "rgba fallback" (5.0.2)20.489
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
android.emulator.Titanium.UI.View"after each" hook for "getOrCreateView() should always return a View" (5.0.2)10.439
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)

Generated by 🚫 dangerJS against 6449b27

Copy link
Contributor

@garymathews garymathews 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

if (hasCustomBackground) {
btn = new AppCompatButton(proxy.getActivity()) {
@Override
public boolean onFilterTouchEventForSecurity(MotionEvent event)
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential to move this out and remove duplicate code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it, but the duplicated code was so small that I didn't think it was worth it. Especially since it would bloat it to the below. 🙄

public boolean onFilterTouchEventForSecurity(MotionEvent event)
{
	boolean isTouchAllowed = super.onFilterTouchEventForSecurity(event);
	return handleFilterTouchEventForSecurity(isTouchAllowed, event);
}

private boolean handleFilterTouchEventForSecurity(boolean isAllowed, MotionEvent event)
{
	if (!isAllowed) {
		fireSyncEvent(TiC.EVENT_TOUCH_FILTERED, dictFromEvent(event));
	}
	return isAllowed;
}

@lokeshchdhry
Copy link
Contributor

FR Passed.

@sgtcoolguy sgtcoolguy merged commit 6649039 into tidev:master Mar 16, 2021
@build
Copy link
Contributor

build commented Mar 16, 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-12537-to-10_0_X
# Apply the commits from the PR
curl -s https://github.com/appcelerator/titanium_mobile/commit/2b34433a34513b4bf0bf6cfd65b3a9ba879c8ebc.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/8da43e7ae1df50bac37b9dfb781d6ba71f330193.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/8858e79cd0ebaa911a7ea5ca7eec74b876f7a740.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/35a656e06353a293364ebcc94a79796ff289c70f.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/216fff45e5a362c77ef5e9974eb7dcb9e75b720f.patch | git am -3 --ignore-whitespace
# Push it to GitHub
git push --set-upstream origin backport-12537-to-10_0_X

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

@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