Widget .text() and .html() don't set .props.text

[notEvil]

var button = ui.addButton('initial', 0, 0, 0.2, 0.1)
button.text('text')
button.props.background = '#ff00ff00'

shows "initial" and not "text".

[notEvil]

There seems to be a general inconsistency between instance methods (e.g. .text, .textSize, .textAlign, .background, ...) and the corresponding styling properties. From my experience, it is mostly sufficient and "better" (e.g. .background) to set the styling properties instead of calling the methods.

I guess this is a consequence of project growth and/or misunderstanding of the concepts on my part.

[victordiaz]

@notEvil you are right, it's not very consistent, mainly because I did not know what I was doing :D
I don't think there is any misunderstanding from your side :)

Originally I had text.background but at some point I realized that it was not very flexible so I introduced the text.props.*** to gather all the styling props in a central place.

It will be great to list here the inconsistencies that you see to have a better sense of the problems.
I also think it will make everything much simpler if there is just 1 way of doing things.

[notEvil]

I don't think there is any misunderstanding from your side :)

After reading the code I have to disagree. Styler defines the general look and feel of widgets and uses the methods to apply the style. And since there is no opt-out, which is perfectly fine, it should be discouraged to use the methods directly. This is a little unfortunate since "content" (e.g. PText/PButton/... text, PToggle on/off text, ...) shouldn't be part of .props and instead set using the methods. New users will set text using .text(), see .textAlign() and think this is the way to go. There are scenarios where this will work, e.g. when .props is not used at all, but it will break as soon as it is.

One such example is #137 where text size and color don't apply, see

PHONK/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/PTextList.java

Line 68 in 3c4024e

styler.apply();

and this issue is due to

PHONK/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/PButton.java

Line 260 in 3c4024e

text(mProps.get("text").toString());

So I would suggest to remove "content" from .props, make sure the code base uses .props everywhere and mark the methods as "low level" in the documentation. What do you think?

[notEvil]

This commit contains a bit more, like .textFont not forgetting about previous .textStyle and fix for

PHONK/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/PButton.java

Line 165 in 3c4024e

this.textSize(size);

(infinite loop if I'm not mistaken). Not tested yet!

[notEvil]

I've added another commit to https://github.com/notEvil/PHONK/tree/style and did some testing using

this

// TODO
// - check box
//   ? implement
// - knob
//   ? remove text interface
// - radio button
//   - figure out how to style child views
// - spinner
//   - figure out how to style this
// - text list
//   - figure out how to style child views

var FIRST_TEXT = 'ERROR'

var view_defs = [
  ['button', (x, y, w, h) => ui.addButton(FIRST_TEXT, x, y, w, h).text('text')],
  ['button 2', (x, y, w, h) => {
    var view = ui.newView('button').text('text')
    ui.addView(view, x, y, w, h)
    return view
  }],
  // ['checkbox', (x, y, w, h) => {
  //   var view = ui.addCheckBox(x, y, w, h)
  //   view.setText('text')
  //   return view
  // }],
  // ['checkbox 2', (x, y, w, h) => {
  //   var view = ui.newView('checkBox')
  //   view.setText('text')
  //   ui.addView(view, x, y, w, h)
  //   return view
  // }],
  ['input', (x, y, w, h) => {
    var view = ui.addInput(x, y, w, h)
    view.setText('text')
    return view
  }],
  ['input 2', (x, y, w, h) => {
    var view = ui.newView('input')
    view.setText('text')
    ui.addView(view, x, y, w, h)
    return view
  }],
  // ['knob', (x, y, w, h) => ui.addKnob(x, y, w, h).text('text')],
  // ['knob 2', (x, y, w, h) => {
  //   var view = ui.newView('knob').text('text')
  //   ui.addView(view, x, y, w, h)
  //   return view
  // }],
  ['text', (x, y, w, h) => ui.addText(FIRST_TEXT, x, y, w, h).text('text')],
  ['text 2', (x, y, w, h) => {
    var view = ui.newView('text').text('text')
    ui.addView(view, x, y, w, h)
    return view
  }],
  ['toggle', (x, y, w, h) => {
    var view = ui.addToggle(FIRST_TEXT, x, y, w, h)
    view.setText('text')
    return view
  }],
  ['toggle 2', (x, y, w, h) => {
    var view = ui.newView('toggle')
    view.setText('text')
    ui.addView(view, x, y, w, h)
    return view
  }],
]

var _ = 0.03 * device.info().screenHeight

var style_defs = [
  ['default', (v) => {  }],
  ['background', (v) => { v.props.background = '#800000ff' }],
  ['borderColor', (v) => { v.props.borderColor = '#ff0000ff' }],
  ['borderRadius', (v) => { v.props.borderRadius = _ }],
  ['borderWidth', (v) => { v.props.borderWidth = _ }],
  ['opacity', (v) => { v.props.opacity = 0.5 }],
  ['padding', (v) => { v.props.padding = _ }],
  ['textAlign', (v) => { v.props.textAlign = 'right' }],
  ['textColor', (v) => { v.props.textColor = '#ff00ff00' }],
  ['textFont', (v) => { v.props.textFont = 'serif' }],
  ['textSize', (v) => { v.props.textSize = 10 }],
  ['textStyle', (v) => { v.props.textStyle = 'italic' }],
]


var Y_MARGIN = 0.01

var view
var y = Y_MARGIN
var h

ui.allowScroll(true)

for (var view_def of view_defs) {
  for (var style_def of style_defs) {
    h = 0.05
    ui.addText(view_def[0] + ', ' + style_def[0], 0.1, y, 0.8, h).props.textAlign = 'center'
    y += h + Y_MARGIN

    h = 0.1
    style_def[1](view_def[1](0.3, y, 0.4, h))
    y += h + Y_MARGIN
  }
}

The code has some flaws, some of which I tried to address

"fixed"

• empty specializations of Styler
• "content" in props ("text", "fit", "hint", "checked", "textOn", "textOff", "srcMode")
• .textFont() ignores previous .textStyle()
• .textStyle() ignores previous .textFont()
• infinite recursion in PButton.textSize()
• .text(), .checked() don't return this
• unused instance variables in Styler and specializations

not "fixed"

• PKnob doesn't use text interface
• PTextList styling doesn't apply to existing child views
• inconsistent code style
  • this.
  • variable naming, e.g. p{widget name}, v, et, tv in PViewsArea
• inconsistent code structure
  • sometimes strange order of methods

Obviously some are off topic, insignificant or not important, but it should make reading the changes easier and provide ground for discussion.

[victordiaz]

Hi! I had a look on the branch, it looks good!

I think it makes sense to strip out all non-related styling props from the props.
Also, do you think renaming
component.props.property to component.style.property would make sense?

Back in the day I started a visual UI editor, which I did not complete because life and bills... Then I thought it would be handy to pass a JSON structure between the editor and the object to set properties such position, label, etc

I found a screenshot somewhere... it was a bit more advanced than this

I think if at some point we would like to continue with the visual Editor it would be better to use another method or property which is not "public", as you suggested somewhere.

If you want to open a PR with the changes I'd be happy to review it and merge it.

[notEvil]

Also, do you think renaming component.props.property to component.style.property would make sense?

Except for "id" and "x", "y", "w", "h", props contains only styling properties. "id" is entirely internal and xywh are only used by PViewsArea.addView. They could be (re)moved but that's not entirely backwards compatible. I wouldn't mind that.

I think if at some point we would like to continue with the visual Editor it would be better to use another method or property which is not "public", as you suggested somewhere.

This is a feature that would make a huge difference in developer experience, at least when using absolute positioning. (I'm planning to add an example for linear layouts soon)

If you want to open a PR with the changes I'd be happy to review it and merge it.

Sure, will do.

[notEvil]

In the PR I made an attempt to remove id, x, y, w, h. For id I've added PViewMethodsInterface.id(). For xywh there is PViewMethodsInterface.set which still uses Styler to do the job, but no interface to get them yet. Maybe .layoutParams() returning getLayoutParams(), and .layoutParams(p) using setLayoutParams()? It would be consistent with other methods that are fully transparent (renames of base methods).

[notEvil]

Ignore the getLayoutParams() suggestion, it wouldn't support units as props.xywh do. Maybe something like

this?

diff --git a/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/LayoutParamsProxy.java b/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/LayoutParamsProxy.java
new file mode 100644
index 0000000..6a3d5f3
--- /dev/null
+++ b/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/LayoutParamsProxy.java
@@ -0,0 +1,39 @@
+package io.phonk.runner.apprunner.api.widgets;
+
+public class LayoutParamsProxy extends Proxy {
+    private Styler styler;
+
+    public LayoutParamsProxy(Styler styler) {
+        this.styler = styler;
+
+        onChange((key, value) -> { change(key, value); });
+    }
+
+    private void change(Object key, Object value) {
+        if (key instanceof String && value != null) {
+            switch ((String) key) {
+                case "x":
+                    styler.setX(value);
+                    break;
+                case "y":
+                    styler.setY(value);
+                    break;
+                case "width":
+                    styler.setWidth(value);
+                    break;
+                case "height":
+                    styler.setHeight(value);
+                    break;
+            }
+        } else {
+            value = get("x");
+            if (value != null) styler.setX(value);
+            value = get("y");
+            if (value != null) styler.setY(value);
+            value = get("width");
+            if (value != null) styler.setWidth(value);
+            value = get("height");
+            if (value != null) styler.setHeight(value);
+        }
+    }
+}
diff --git a/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/PButton.java b/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/PButton.java
index 44d8c4b..b47a8cc 100644
--- a/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/PButton.java
+++ b/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/PButton.java
@@ -46,6 +46,7 @@ public class PButton extends androidx.appcompat.widget.AppCompatButton implement
 
     // this is a props proxy for the user
     public final StylePropertiesProxy props = new StylePropertiesProxy();
+    public LayoutParamsProxy layoutParams;
 
     // the props are transformed / accessed using the styler object
     public final Styler styler;
@@ -67,6 +68,8 @@ public class PButton extends androidx.appcompat.widget.AppCompatButton implement
         Styler.fromTo(initProps, props);
         props.eventOnChange = true;
         styler.apply();
+
+        layoutParams = new LayoutParamsProxy(styler);
     }
 
     @SuppressLint("ClickableViewAccessibility")
diff --git a/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/Proxy.java b/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/Proxy.java
new file mode 100644
index 0000000..645a3c9
--- /dev/null
+++ b/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/Proxy.java
@@ -0,0 +1,64 @@
+package io.phonk.runner.apprunner.api.widgets;
+
+import io.phonk.runner.apprunner.api.common.ReturnObject;
+import java.util.Map;
+import org.mozilla.javascript.Scriptable;
+
+public class Proxy extends ReturnObject {
+    private OnChangeListener changeListener;
+    public boolean eventOnChange = true;
+
+    @Override
+    public Object put(Object key, Object value) {
+        Object previousValue = super.put(key, value);
+        changed(key, value);
+        return previousValue;
+    }
+
+    @Override
+    public void put(String name, Scriptable start, Object value) {
+        super.put(name, start, value);
+        changed(name, value);
+    }
+
+    @Override
+    public void put(int index, Scriptable start, Object value) {
+        super.put(index, start, value);
+        changed(index, value);
+    }
+
+    @Override
+    public void putAll(Map m) {
+        super.putAll(m);
+        changed(null, null);
+    }
+
+    @Override
+    public void delete(String name) {
+        super.delete(name);
+        changed(name, null);
+    }
+
+    @Override
+    public void delete(int index) {
+        super.delete(index);
+        changed(index, null);
+    }
+
+    public interface OnChangeListener {
+        void event(Object key, Object value);
+    }
+
+    public Proxy onChange(OnChangeListener changeListener) {
+        this.changeListener = changeListener;
+        return this;
+    }
+
+    public void changed() {
+        changed(null, null);
+    }
+
+    private void changed(Object key, Object value) {
+        if (changeListener != null && eventOnChange) changeListener.event(key, value);
+    }
+}
diff --git a/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/Styler.java b/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/Styler.java
index 29697f2..3392695 100644
--- a/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/Styler.java
+++ b/PHONK-android/phonk_apprunner/src/main/java/io/phonk/runner/apprunner/api/widgets/Styler.java
@@ -293,7 +293,7 @@ public class Styler {
         }
     }
 
-    private void setWidth(Object value) {
+    public void setWidth(Object value) {
         getScreenSize();
         int val = mAppRunner.pUtil.sizeToPixels(value, mWidth);
 
@@ -304,7 +304,7 @@ public class Styler {
         }
     }
 
-    private void setHeight(Object value) {
+    public void setHeight(Object value) {
         getScreenSize();
         int val = mAppRunner.pUtil.sizeToPixels(value, mHeight);
 

Proxy is very similar to StylePropertiesProxy and could replace it for the most part.

[victordiaz]

what you are proposing here is having a separation of concerns. The same way we do with the style properties but this time with position and sizing, isnt it?

[notEvil]

Yes, but the proxy could also be more general, the new ".props" so to speak. That way, it could contain id and user defined objects. In general, having a separate proxy for style and a common proxy type is a good thing imo.

[victordiaz]

That sounds good. It will pave the path for the visual UI editor.
If you think is worth it we could give it a try.