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): menu and toolbar icons to use ActionBar style colors #13147

Merged
merged 2 commits into from Oct 29, 2021

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Oct 27, 2021

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

Summary:
When setting menu/toolbar icons to a resource ID (ie: a vector drawable), the icon should use the theme/style assigned to the ActionBar or Toolbar. Currently, it uses the style colors assigned to the activity window, which won't be the same as the ActionBar if using a DarkActionBar theme.

Test:

  1. Download chore: update for Titanium SDK 10.1.0 kitchensink-v2#59
  2. Build and run on Android.
  3. Make sure device is using the Light theme.
  4. Go to: Views -> ListView (Multi-Select)
  5. Verify all top toolbar icons are white. (They used to be gray.)

@build
Copy link
Contributor

build commented Oct 27, 2021

Warnings
⚠️ 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 1157 skipped out of 20813 total tests.
📖

💾 Here's the generated SDK zipfile.

Tests:

ClassnameNameTimeError
android.emulator.5.0.Titanium.UI.View"after each" hook for "getOrCreateView() should always return a View" (5.0.2)10.621
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.main.Titanium.Geolocation.methods#forwardGeocoder() works via callback argument (12)5.061
Error: expected false to be true
at Assertion.fail (/node_modules/should/cjs/should.js:275:13)
      at Assertion.value (/node_modules/should/cjs/should.js:356:9)
      at Geolocation.<anonymous> (/ti.geolocation.test.js:585:32)

Generated by 🚫 dangerJS against bb0e605

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
FT: PASS

@jquick-axway jquick-axway merged commit 2052f78 into tidev:master Oct 29, 2021
@hansemannn
Copy link
Collaborator

hansemannn commented Apr 19, 2022

I wonder if this change caused the [ERROR] E/com.example.myapp Invalid ID 0x00000000. error log. I was able to reproduce it with just these few lines of code:

<Alloy>
	<Window>
		<Menu platform="android">
			<MenuItem title="Test" />
		</Menu>
	</Window>
</Alloy>

cc @m1ga, you may have seen the error log as well.

EDIT: It seems like the issue is a missing itemId property which is not auto-generated by default anymore. Also, setting it to an already existing ID makes it invisible. So as a workaround, a unique number for every menu item should do it.

@m1ga
Copy link
Contributor

m1ga commented Apr 19, 2022

I'll check that 👍

@m1ga
Copy link
Contributor

m1ga commented Apr 19, 2022

@hansemannn it's strange. I can't reproduce it at the moment with 10.2.0. Neither with the demo code nor with my app 🤔 I had this for a long time but now I don't see it anymore. I'll give it another try later today

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