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): problems with "userinterfacestyle" event #13265

Merged
merged 3 commits into from
Mar 27, 2022

Conversation

drauggres
Copy link
Contributor

use TiApplication.onConfigurationChanged instead of BroadcastReceiver

use TiApplication.onConfigurationChanged instead of BroadcastReceiver

fix tidev#13264
@hansemannn
Copy link
Collaborator

Does this potentially fix https://jira.appcelerator.org/browse/TIMOB-28563??

@drauggres
Copy link
Contributor Author

This patch fixes only issues with userinterfacestyle (not "override") that exists only Android API 31+.

With the patch, applied I can reproduce these two:

  • Some system colors like the action bar title are correctly changed
  • Some other colors (semantic colors set via semantic.colors.json) are not changed
Extended app sample
let eventCounter = 0;
let changeCounter = 0;
let lastStyle = getUiStyleName(Ti.UI.userInterfaceStyle)

const win = Ti.UI.createWindow({
backgroundColor: 'primaryBackground'
});

const wrapper = Ti.UI.createView({
layout: 'vertical',
width: Ti.UI.SIZE,
height: Ti.UI.SIZE,
center: {
  x: '50%',
  y: '50%'
}
})

const eventLabel = Ti.UI.createLabel({
text: `style after start: ${lastStyle}`,
color: 'label',
font: {
  fontSize: 20
}
});

const eventCounterLabel = Ti.UI.createLabel({
text: `userinterfacestyle events: ${eventCounter}`,
color: 'label',
font: {
  fontSize: 20
}
});

const changeCounterLabel = Ti.UI.createLabel({
text: `style changes: ${changeCounter}`,
color: 'label',
font: {
  fontSize: 20
}
});

const intervalLabel = Ti.UI.createLabel({
text: ` Ti.UI.userInterfaceStyle: ${lastStyle}`,
color: 'label',
font: {
  fontSize: 20
}
});

const overrideDarkButton = Ti.UI.createButton({
title: 'Force dark'
});
overrideDarkButton.addEventListener('click', function() {
Ti.UI.overrideUserInterfaceStyle = Ti.UI.USER_INTERFACE_STYLE_DARK
})
const overrideLightButton = Ti.UI.createButton({
title: 'Force light'
});
overrideLightButton.addEventListener('click', function() {
Ti.UI.overrideUserInterfaceStyle = Ti.UI.USER_INTERFACE_STYLE_LIGHT
})
const overrideRemoveButton = Ti.UI.createButton({
title: 'Remove override '
});
overrideRemoveButton.addEventListener('click', function() {
Ti.UI.overrideUserInterfaceStyle = Ti.UI.USER_INTERFACE_STYLE_UNSPECIFIED
})

wrapper.add(eventLabel);
wrapper.add(intervalLabel);
wrapper.add(eventCounterLabel);
wrapper.add(changeCounterLabel)
wrapper.add(overrideDarkButton);
wrapper.add(overrideLightButton);
wrapper.add(overrideRemoveButton);

win.add(wrapper);
win.open();

Ti.UI.addEventListener('userinterfacestyle', function onUiStyle(event) {
eventLabel.text = `style from event: ${getUiStyleName(event.value)}`;
eventCounterLabel.text = `userinterfacestyle events: ${++eventCounter}`;
});

setInterval(function() {
const current = getUiStyleName(Ti.UI.userInterfaceStyle);
if (current !== lastStyle) {
  intervalLabel.text = ` Ti.UI.userInterfaceStyle: ${current}`;
  lastStyle = current;
  changeCounterLabel.text = `style changes: ${++changeCounter}`
}
}, 500);


function getUiStyleName(mode) {
if (mode === Ti.UI.USER_INTERFACE_STYLE_DARK) {
  return 'Dark';
} else if (mode === Ti.UI.USER_INTERFACE_STYLE_LIGHT) {
  return 'Light';
} else {
  return 'Undefined';
}
}

P.S. I stopped creating/checking ticket in the JIRA.

drauggres added a commit to drauggres/titanium_mobile that referenced this pull request Feb 18, 2022
@m1ga
Copy link
Contributor

m1ga commented Mar 26, 2022

with the artifact 10.2.0.v20220321092109 it doesn't work for me:

  • changing system dark/light mode works fine (background is changing color)
  • clicking the force button doesn't change the background color:
screencapture-2022-03-26_20.49.32.mp4

I'll build from source tomorrow and test it again

@drauggres
Copy link
Contributor Author

@m1ga fix for overrideUserInterfaceStyle (#13267) was merged yesterday (2022 03 25), i.e. "force" buttons should be broken in this build.

I've merged master manually. I don't know why, but there is no Update branch button (maybe it is still disabled in settings, or something got broken after repo transferring).

@m1ga
Copy link
Contributor

m1ga commented Mar 26, 2022

Yeah, the "out of date - merge" button was really handy! I'll test the new build later, didn't check the merge times before 👍

@m1ga
Copy link
Contributor

m1ga commented Mar 26, 2022

Don't see any difference between master and this PR. Both change the actionbar and all colors. Tested it on Android 12 (device) and 13 (emulator).
Is there any way to reproduce the problem @drauggres ?

@drauggres
Copy link
Contributor Author

drauggres commented Mar 26, 2022

Both change the actionbar and all colors.

As the title states, problem is with the userinterfacestyle event, not the colors.

Is there any way to reproduce the problem @drauggres ?

I can reproduce it every time:

  • start the app
  • change the style
  • there is no userinterfacestyle event
  • change style back
  • userinterfacestyle event recieved with wrong value

Screenshot are made on Android Emulator API 32.
UPD: I can reproduce the same on Google Pixel 3 XL with Android 12.

master

Screenshot_1648331401

master video
master.mp4
PR

Screenshot_1648331331

@m1ga
Copy link
Contributor

m1ga commented Mar 27, 2022

Ok, it will go out of sync at one point. Sometime after 3 switches sometimes quicker. I've managed to record a video where it happend at the second time:

without PR (master 10.2.0):

screencapture-2022-03-27_11.18.57.mp4

with PR:

screencapture-2022-03-27_11.20.53.mp4

Tested the PR with my app and saw no issues. Looks fine to me 👍 @hansemannn

@hansemannn
Copy link
Collaborator

Thx! Btw, you can use the "Approve" functionality in GitHub to give emphasis to the approval.

@hansemannn hansemannn merged commit bdfd640 into tidev:master Mar 27, 2022
@hansemannn
Copy link
Collaborator

@drauggres You rock!

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.

3 participants