victordiaz / PHONK

PHONK is a coding playground for new and old Android devices
https://phonk.app
GNU General Public License v3.0
457 stars 27 forks source link

Partial revert of #141 and improved properties #142

Closed notEvil closed 1 year ago

notEvil commented 1 year ago

Hi,

I didn't consider https://github.com/victordiaz/PHONK/pull/141 final (https://github.com/victordiaz/PHONK/issues/139#issuecomment-1464188735) so this PR partially reverts its changes.

Instead of having separate containers for id, style, layout(, state) as proposed, I found that improving the way changes are applied already provides better separation and fits previous conventions. Essentially

To review this, it is probably better to compare with https://github.com/victordiaz/PHONK/commit/3c4024e447bdde2615403eaa3fc3e65005886590 and consider it a replacement for https://github.com/victordiaz/PHONK/pull/141 which I consider inferior and less backwards compatible. Sorry!

If you agree, I would also add "content" (text, checked, ...) back to props.

Changes

notEvil commented 1 year ago

Hope you don't mind the regression fix for https://github.com/victordiaz/PHONK/issues/137 and fix for https://github.com/victordiaz/PHONK/issues/140 in here. The former is related insofar as the bug was introduced in the previous PR and the latter improves PLinearLayout with respect to properties w, h.

victordiaz commented 1 year ago

Hi! Sorry for merging the previous PR too quickly. I thought it was ready to roll :)

I just had a quick read on the changes and this direction seems right to me.

If you agree, I would also add "content" (text, checked, ...) back to props. Yes, that would be great!

I will refrain my self to approve and merge it too quickly this time :)

notEvil commented 1 year ago

nP :)

The PR is ready to test and review now. I ran the GUI examples and my application, and they seem fine. The commit messages should summarize the changes for an easier review. If you have any doubts, let me know!

notEvil commented 1 year ago

Great :)

Just for reference:

So it might be worthwhile to revisit each widget and see if it fullfills its purpose and has all relevant properties. If you would write a list of things to improve, I'd certainly contribute! :)

The only think I might change at some point in the future is the example

I agree, its too complicated.

victordiaz commented 1 year ago

I noticed the removed code. Thanks for doing it. Many things were intentions that never resolved :) So better removed for clarity.

Regarding the tasks, there is 4 things that I wanted to have since long time

1) a visual UI builder. 2) some machine learning for vision stuff. There is some working code already for training and detecting images using the camera. The pace of AI models is so fast that probably feels quite obsolete already. For this, the first action would be replacing the PCamera code which uses the old Camera API to CameraX API. Then see if it makes sense to keep using Tensorflow, Mediapipe or whatever new solution is there. I have some scripts working in case you are interested 3) APK creation via an Android template where you can place your user scripts and build an standalone APK. This is something that worked long time ago but it got lost. Many people asked for this but I never found the time to get it done :/ If you see the project it is separated into 2 parts. The App and the AppRunner. So basically is just creating an Android module that loads an script and uses the AppRunner to execute it. 4) Better documentation. I know you did some code parsing already :)

If there is something that interests you from this list (or something else) I'd be more than happy to support you in any way.

notEvil commented 1 year ago

I meant improvements regarding widgets and layout, since this is where I can reuse the knowledge I have atm. For the other things I fear my knowledge (e.g. vue, ML on Android, app bundling) doesn't suffice and their absence don't bug me enough to commit to it. The documentation however is an area where you might see a PR in the future! (didn't start yet)