Migrate application to compose

[FunkyMuse]

• RadioGroupDialog

I've started working on migrating RadioGroupDialog to Compose btw.

[esensar]

• RadioGroupDialog

I've started working on migrating RadioGroupDialog to Compose btw.

Okay, great! Haven't started on that one yet, I have been working on the one defined in this app, so once you finish that, it will come in handy here.

[FunkyMuse]

• RadioGroupDialog

I've started working on migrating RadioGroupDialog to Compose btw.

Okay, great! Haven't started on that one yet, I have been working on the one defined in this app, so once you finish that, it will come in handy here.

I can see SleepTimerCustomDialog is not existent in Commons, you can take that one.

For ColorPickerDialog i think it might be best to incorporate an existing Compose library @tibbi

[tibbi]

we can check some library if it works similarly as the current one, by allowing comparison of current and new color and inserting new hexcode manually

[FunkyMuse]

we can check some library if it works similarly as the current one, by allowing comparison of current and new color and inserting new hexcode manually

We can fork this one and add ability to insert hexcode manually https://github.com/skydoves/colorpicker-compose

@esensar what do you think?

[esensar]

we can check some library if it works similarly as the current one, by allowing comparison of current and new color and inserting new hexcode manually

We can fork this one and add ability to insert hexcode manually https://github.com/skydoves/colorpicker-compose

@esensar what do you think?

Sounds good to me. I will just try including it in the project locally to see if it works alright.

[esensar]

we can check some library if it works similarly as the current one, by allowing comparison of current and new color and inserting new hexcode manually

We can fork this one and add ability to insert hexcode manually https://github.com/skydoves/colorpicker-compose

@esensar what do you think?

I think we can do it without forking, since it is split into all of these separate components, and the controller is separate from the view, which allows us to change selected values manually.

[esensar]

we can check some library if it works similarly as the current one, by allowing comparison of current and new color and inserting new hexcode manually

We can fork this one and add ability to insert hexcode manually https://github.com/skydoves/colorpicker-compose @esensar what do you think?

I think we can do it without forking, since it is split into all of these separate components, and the controller is separate from the view, which allows us to change selected values manually.

Do you want to pick it up @FunkyMuse or do you want me to make a PR? I think with this library we could rebuild similar layout to the existing one, but in Compose and I don't think we would need to fork it since it is pretty modular and easy to modify from outside.

[FunkyMuse]

we can check some library if it works similarly as the current one, by allowing comparison of current and new color and inserting new hexcode manually

We can fork this one and add ability to insert hexcode manually https://github.com/skydoves/colorpicker-compose @esensar what do you think?

I think we can do it without forking, since it is split into all of these separate components, and the controller is separate from the view, which allows us to change selected values manually.

Do you want to pick it up @FunkyMuse or do you want me to make a PR? I think with this library we could rebuild similar layout to the existing one, but in Compose and I don't think we would need to fork it since it is pretty modular and easy to modify from outside.

You can take that one

[esensar]

I think this is finally ready for testing, unless I missed something :smile:

@FunkyMuse would you mind doing one more review?

[FunkyMuse]

I think this is finally ready for testing, unless I missed something 😄

@FunkyMuse would you mind doing one more review?

I'm on it, give me some time for detailed review.

[esensar]

Waiting on https://github.com/SimpleMobileTools/Simple-Commons/pull/1868 and https://github.com/SimpleMobileTools/Simple-Commons/pull/1869 for more compose stuff and fixes for widget configuration activities backgrounds (right now it is not transparent).

[FunkyMuse]

Purchase dialog is always shown when trying to add a widget, clicking purchase and coming back just allows me to add the widget anyway

[FunkyMuse]

The main screen is not scrollable in portrait mode (when forced through settings) and it's missing a toolbar

[FunkyMuse]

MainActivity is still using appLaunched(BuildConfig.APPLICATION_ID) which is the old way, it should use the new Compose way, check simple commons

[esensar]

The main screen is not scrollable in portrait mode (when forced through settings) and it's missing a toolbar

I will add scrolling, but toolbar was never there in this app. I hope I understood that right :smile:

[FunkyMuse]

The main screen is not scrollable in portrait mode (when forced through settings) and it's missing a toolbar

I will add scrolling, but toolbar was never there in this app. I hope I understood that right 😄

It's in the app, rotate the device you'll see it

[esensar]

The main screen is not scrollable in portrait mode (when forced through settings) and it's missing a toolbar

I will add scrolling, but toolbar was never there in this app. I hope I understood that right 😄

It's in the app, rotate the device you'll see it

I can see it only after I scroll down. Now that I added scrolling to the app, it behaves that way. Is that alright?

[FunkyMuse]

The main screen is not scrollable in portrait mode (when forced through settings) and it's missing a toolbar

I will add scrolling, but toolbar was never there in this app. I hope I understood that right 😄

It's in the app, rotate the device you'll see it

I can see it only after I scroll down. Now that I added scrolling to the app, it behaves that way. Is that alright?

Yes only on scrolling, just like About screen, Settings etc.. I'll check in a bit.

[esensar]

Waiting for https://github.com/SimpleMobileTools/Simple-Commons/pull/1872 to fix purchase dialog in widget configuration

[FunkyMuse]

When I press on the flashlight, the progress bar jumps from zero to full the first time it's shown

[FunkyMuse]

We should definitely modify the background color of the three vertical dots menu, in dark theme it's barely visible

and tweak the text color if needed when the BG is different

[esensar]

We should definitely modify the background color of the three vertical dots menu, in dark theme it's barely visible

and tweak the text color if needed when the BG is different

Which settings did you use to get that? I got that once, but I forgot which color setup and device theme I had. I can't reproduce it now.

[FunkyMuse]

We should definitely modify the background color of the three vertical dots menu, in dark theme it's barely visible and tweak the text color if needed when the BG is different

Which settings did you use to get that? I got that once, but I forgot which color setup and device theme I had. I can't reproduce it now.

Black/White theme

[FunkyMuse]

We should definitely modify the background color of the three vertical dots menu, in dark theme it's barely visible and tweak the text color if needed when the BG is different

Which settings did you use to get that? I got that once, but I forgot which color setup and device theme I had. I can't reproduce it now.

Black/White theme

@esensar after discussing with @tibbi it was decided that in Black and White theme, the ActionMenu should behave the same as the AlertDialog's Compose background, the BG is black, the text is white and there's a border, we can just reuse that part

[FunkyMuse]

when I click cancel when the sleep timer asks for a permission it launches the interval picker anyways and the user can start the sleep timer even when the clicks cancel but picks an interval

[FunkyMuse]

also why do we still have the workaround with minActiveState = Lifecycle.State.CREATED?

[FunkyMuse]

even when an error occurs when turning the flashlight ON, the button is in the ON state, it should be disabled

[esensar]

when I click cancel when the sleep timer asks for a permission it launches the interval picker anyways and the user can start the sleep timer even when the clicks cancel but picks an interval

Yeah, that is intended actually. It is also how the app behaved before this migration, but yeah, it is a bit confusing that the button says "Cancel". I should probably update it to something more clear.

Anyways, the reason this happens is that we didn't want to block the users from setting a sleep timer, since it should still work in many cases as expected, especially for shorter timers.

[FunkyMuse]

when I click cancel when the sleep timer asks for a permission it launches the interval picker anyways and the user can start the sleep timer even when the clicks cancel but picks an interval

Yeah, that is intended actually. It is also how the app behaved before this migration, but yeah, it is a bit confusing that the button says "Cancel". I should probably update it to something more clear.

Anyways, the reason this happens is that we didn't want to block the users from setting a sleep timer, since it should still work in many cases as expected, especially for shorter timers.

Yes I know and I agree that cancel is a bit not descriptive @tibbi we can continue or make the button more descriptive?

[esensar]

also why do we still have the workaround with minActiveState = Lifecycle.State.CREATED?

I never saw your response on the original question. I just saw it now and tried it. I still have the same issue.

I think the issue is that, this preferences.flow thing is only observing changes in the preferences and in this case, they happen while this activity is stopped, since the user is on SettingsActivity at that time. Since creation didn't happen, it was only going to onStart again, the observing just continues and it ignores the current value.

Maybe we should change sharedPreferencesCallback to publish current value as soon as collection on that flow is started?

[FunkyMuse]

also why do we still have the workaround with minActiveState = Lifecycle.State.CREATED?

I never saw your response on the original question. I just saw it now and tried it. I still have the same issue.

I think the issue is that, this preferences.flow thing is only observing changes in the preferences and in this case, they happen while this activity is stopped, since the user is on SettingsActivity at that time. Since creation didn't happen, it was only going to onStart again, the observing just continues and it ignores the current value.

Maybe we should change sharedPreferencesCallback to publish current value as soon as collection on that flow is started?

Why others work and some doesn't?

[esensar]

also why do we still have the workaround with minActiveState = Lifecycle.State.CREATED?

I never saw your response on the original question. I just saw it now and tried it. I still have the same issue. I think the issue is that, this preferences.flow thing is only observing changes in the preferences and in this case, they happen while this activity is stopped, since the user is on SettingsActivity at that time. Since creation didn't happen, it was only going to onStart again, the observing just continues and it ignores the current value. Maybe we should change sharedPreferencesCallback to publish current value as soon as collection on that flow is started?

Why others work and some doesn't?

Only these with that minActiveState are changed from outside this activity.

[FunkyMuse]

also why do we still have the workaround with minActiveState = Lifecycle.State.CREATED?

I never saw your response on the original question. I just saw it now and tried it. I still have the same issue. I think the issue is that, this preferences.flow thing is only observing changes in the preferences and in this case, they happen while this activity is stopped, since the user is on SettingsActivity at that time. Since creation didn't happen, it was only going to onStart again, the observing just continues and it ignores the current value. Maybe we should change sharedPreferencesCallback to publish current value as soon as collection on that flow is started?

Why others work and some doesn't?

Only these with that minActiveState are changed from outside this activity.

Yes that's correct, the Lifecycle is different, maybe we should create as you've suggested a different helper that spits out the value immediately when a collector is listening.

Also the current one has a Channel backing that doesn't allow more than 1 collector FYI.

[FunkyMuse]

please solve the conflicts

[FunkyMuse]

@tibbi on my end (code wise) this looks good, if you have nothing to add we can merge

[tibbi]

widget config screen is really broken, top status bar is invisible and OK is under the navigation bar

[tibbi]

Custom sleep timer dialog looks really bad too

[tibbi]

and the sleep timer remaining time overlaps with the stroboscope frequency and flashlight brightness bar

[tibbi]

this is actually quite far from done, not sure if it makes sense to continue, if we can make it done in time at all...

[FunkyMuse]

this is actually quite far from done, not sure if it makes sense to continue, if we can make it done in time at all...

how much time do we have?

[tibbi]

maybe by wednesday it should be ready, with all themes, dialogs and screens tested properly

[FunkyMuse]

maybe by wednesday it should be ready, with all themes, dialogs and screens tested properly

@esensar want to take it or should i make the remaining changes?

[esensar]

maybe by wednesday it should be ready, with all themes, dialogs and screens tested properly

@esensar want to take it or should i make the remaining changes?

Go ahead. We can also split it among us to get them done faster.

[FunkyMuse]

maybe by wednesday it should be ready, with all themes, dialogs and screens tested properly

@esensar want to take it or should i make the remaining changes?

Go ahead. We can also split it among us to get them done faster.

I can't code ATM as I'm traveling, i can review your changes, sorry 😐

[esensar]

maybe by wednesday it should be ready, with all themes, dialogs and screens tested properly

@esensar want to take it or should i make the remaining changes?

Go ahead. We can also split it among us to get them done faster.

I can't code ATM as I'm traveling, i can review your changes, sorry 😐

No problem, I took care of the smaller ones mentioned. I will keep testing for other issues.

[esensar]

widget config screen is really broken, top status bar is invisible and OK is under the navigation bar

I am not sure what you mean by that top status bar thing. Do you want it to have background? I noticed it had it before, but now the wallpaper is fully displayed in this compose version. I fixed the OK button placement issue.

[FunkyMuse]

Code wise looks good again

[tibbi]

widget config screen is really broken, top status bar is invisible and OK is under the navigation bar

I am not sure what you mean by that top status bar thing. Do you want it to have background? I noticed it had it before, but now the wallpaper is fully displayed in this compose version. I fixed the OK button placement issue.

well, in light Material You theme I cannot see the status bar at all, it is like black on black or so

[tibbi]

alright thanks guys, seems to be working fine now, will see the tests on Google Play