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): amend actionBarStyle for MaterialComponents #12908

Closed
wants to merge 1 commit into from

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Jun 18, 2021

  • Amend actionBarStyle to use MaterialComponents instead of AppCompat
TEST CASE
const window = Ti.UI.createWindow();

window.addEventListener('open', _ => {
	const actionBar = window.activity.actionBar;

	if (actionBar) {
		actionBar.title = 'ActionBar';
		actionBar.homeButtonEnabled = true;
		actionBar.displayHomeAsUp = true;
		actionBar.onHomeIconItemSelected = _ => {
			alert('Home');
		};
	}
});

window.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 Jun 18, 2021
@garymathews garymathews added this to the 10.1.0 milestone Jun 18, 2021
@build
Copy link
Contributor

build commented Jun 18, 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.
📖 ❌ 1 tests have failed There are 1 tests failing and 961 skipped out of 15379 total tests.
📖

💾 Here's the generated SDK zipfile.

Tests:

ClassnameNameTimeError
ios.macos.Titanium.Blobimage dimensions should be reported in pixels (10.15.7)0.053
Error: expected 6 to be 11
value@file:///Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-12908/tmp/mocha/build/iphone/build/Products/Debug-maccatalyst/mocha.app/Contents/Resources/node_modules/should/cjs/should.js:356:23
postlayout@file:///Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-12908/tmp/mocha/build/iphone/build/Products/Debug-maccatalyst/mocha.app/Contents/Resources/ti.blob.test.js:488:33

Generated by 🚫 dangerJS against 7d092ef

@jquick-axway
Copy link
Contributor

@garymathews, the reason I used "Widget.AppCompat.Light.ActionBar.Solid" for the "actionBarStyle" was to work-around an ActionBar issue where the "subtitle" text would be clipped when shown in landscape. The ActionBar is expected to be shorter in landscape and taller in portrait, but Google's built-in themes mishandles this as can be seen here...
material-components/material-components-android#779

So, I fixed the landscape ActionBar height issue by customizing it myself just like how it used to work.
https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/res/values-land/values.xml
https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/res/values/values.xml#L19

But Google's material ActionBar style sets min heights regardless of orientation which will cause a problem withe shorter landscape height. However, the AppCompat style does correctly compensate for landscape which works-around the problem.
https://github.com/material-components/material-components-android/blob/master/lib/java/com/google/android/material/appbar/res/values/styles.xml#L98-L104

I didn't write an XML comment about this. So, it's my fault for not clarifying why it was set up this way.


Test Code:

const window = Ti.UI.createWindow();
window.addEventListener("open", () => {
	const actionBar = window.activity.actionBar;
	actionBar.title = "My Title";
	actionBar.subtitle = "My Subtitle";
	actionBar.displayHomeAsUp = true;
	actionBar.homeButtonEnabled = true;
	actionBar.onHomeIconItemSelected = () => {
		alert("Hello World!");
	};
});
window.activity.onCreateOptionsMenu = function(e) {
	var menuItem = e.menu.add({
		title: "Menu Item 1",
		showAsAction: Ti.Android.SHOW_AS_ACTION_IF_ROOM,
	});
	menuItem.checked = true;
	console.log("@@@ menuItem.checked: " + menuItem.checked);
	menuItem.addEventListener("click", function() {
		const dialog = Ti.UI.createAlertDialog({
			title: "Alert",
			message: "Menu item was clicked on.",
			buttonNames: ["OK"],
		});
		dialog.show();
	});
};
window.open();

Style "Widget.MaterialComponents.Light.ActionBar.Solid" mishandles text in landscape...
LandscapeBad

Style "Widget.AppCompat.Light.ActionBar.Solid" correctly handles portrait/landscape text sizing...
LandscapeGood

@hansemannn
Copy link
Collaborator

Since the subtitle issue is not resolved so far, I would leave the API as-is for now. Hopefully, this is resolved in future Material Components updates!

@hansemannn hansemannn closed this Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge bug no tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants