[GH-ISSUE #130] Missing features in Area #76

Closed
opened 2026-05-05 11:38:14 -06:00 by gitea-mirror · 31 comments
Owner

Originally created by @mischnic on GitHub (May 16, 2018).
Original GitHub issue: https://github.com/kusti8/proton-native/issues/130

  • scrolling area
  • Gradients
  • Clipping
  • getting the Area's size in pixel (added in #174)
  • make a handle to move and resize the window (UiArea.beginWindowMove & UiArea.beginWindowResize)
Originally created by @mischnic on GitHub (May 16, 2018). Original GitHub issue: https://github.com/kusti8/proton-native/issues/130 - [x] scrolling area - [x] Gradients - [ ] Clipping - [x] getting the Area's size in pixel (added in #174) - [ ] make a handle to move and resize the window (`UiArea.beginWindowMove` & `UiArea.beginWindowResize`)
gitea-mirror 2026-05-05 11:38:14 -06:00
Author
Owner

@mischnic commented on GitHub (Jul 18, 2018):

getting the Area's size in pixel

@kusti8 How can this be done from a API design perspective? There is no draw callback in declarative React code. The only way I can think of is "function as children" ("render callback"):

<Area>
  {(width, height) => <Area.Rectangle ... > .... </Area.Rectangle>}
</Area>

Is that information even needed or is it enough to have width="100%"?

<!-- gh-comment-id:405937159 --> @mischnic commented on GitHub (Jul 18, 2018): > getting the Area's size in pixel @kusti8 How can this be done from a API design perspective? There is no `draw` callback in declarative React code. The only way I can think of is "function as children" ("render callback"): ```jsx <Area> {(width, height) => <Area.Rectangle ... > .... </Area.Rectangle>} </Area> ``` Is that information even needed or is it enough to have `width="100%"`?
Author
Owner

@mischnic commented on GitHub (Jul 18, 2018):

I went with the second option


And regarding gradients: how should they be declared?
1.

const g1 = new Area.Gradient("linear", x1, y1, x2, y2); // start & end coordinates
g1.addStop(0.0, "red");
g1.addStop(1.0, "blue");

// ...

  <Area.Rectangle ... fill={g1}/>
  1. (if that's even possible?)
const g1 = (
<Area.Gradient x1="0" x2="0" x2="100" y2="100">
  <Area.Gradient.Stop color="red" offset="0%"/>
  <Area.Gradient.Stop color="blue" offset="100%"/>
</Area.Gradient>);

// ...

  <Area.Rectangle ... fill={g1}/>
<!-- gh-comment-id:405965923 --> @mischnic commented on GitHub (Jul 18, 2018): **I went with the second option** --- And regarding gradients: how should they be declared? 1. ```jsx const g1 = new Area.Gradient("linear", x1, y1, x2, y2); // start & end coordinates g1.addStop(0.0, "red"); g1.addStop(1.0, "blue"); // ... <Area.Rectangle ... fill={g1}/> ``` 2. (if that's even possible?) ```jsx const g1 = ( <Area.Gradient x1="0" x2="0" x2="100" y2="100"> <Area.Gradient.Stop color="red" offset="0%"/> <Area.Gradient.Stop color="blue" offset="100%"/> </Area.Gradient>); // ... <Area.Rectangle ... fill={g1}/> ```
Author
Owner

@kusti8 commented on GitHub (Jul 22, 2018):

Yeah sorry about the delay. The second option is more like the React syntax. And as for the area size, I agree that a function child is the only real way.

<!-- gh-comment-id:406868708 --> @kusti8 commented on GitHub (Jul 22, 2018): Yeah sorry about the delay. The second option is more like the React syntax. And as for the area size, I agree that a function child is the only real way.
Author
Owner

@mischnic commented on GitHub (Jul 22, 2018):

And as for the area size, I agree that a function child is the only real way.

I'm struggling getting that to work.
First, there is always this warning:

Warning: Functions are not valid as a React child. This may happen if you return a Component instead of <Component /> from render. Or maybe you meant to call this function rather than return it.
    in AREAGROUP (created by Area)
    in AREA (created by Area)
    in Area (created by Example)
    in VERTICALBOX (created by Box)
    in Box (created by Example)
    in WINDOW (created by Example)
    in APP (created by Example)
    in Example

The function gets passed to Area.Group as a child:

Area.Group = class AreaGroup extends AreaComponent {
  constructor(root, props) {
    super(root, props);
    this.children = [];
  }

  appendChild(child) {
    this.children.push(child);
  }

  draw(area, p, props) {
    if(typeof this.props.children === "function"){
      this.appendChild(this.props.children(p.getAreaWidth(), p.getAreaHeight()));
    }
      
    for (let i = 0; i < this.children.length; i += 1) {
      if (typeof this.children[i] === 'object') {
        console.log(this.children[i]);
        this.children[i].render(this, area, p, props);
      }
    }
  }
};

The react element is printed to the console (returned by React.createElement via the function child):

{ '$$typeof': Symbol(react.element),
  type: 'AREATEXT',
  key: null,
  ref: null,
  props: { children: '560 460' },
  _owner: null,
  _store: {} }

But I need something like (the real class instance) this for the rendering:

AreaText {
  root: Root { children: [ [App] ] },
  props:
   { children:
      'Some text',
     x: 0,
     y: 0 },
  element: {},
  children:
   [ 'Some Text' ],
  str: AttributedString {} }
<!-- gh-comment-id:406871836 --> @mischnic commented on GitHub (Jul 22, 2018): > And as for the area size, I agree that a function child is the only real way. I'm struggling getting that to work. First, there is always this warning: ``` Warning: Functions are not valid as a React child. This may happen if you return a Component instead of <Component /> from render. Or maybe you meant to call this function rather than return it. in AREAGROUP (created by Area) in AREA (created by Area) in Area (created by Example) in VERTICALBOX (created by Box) in Box (created by Example) in WINDOW (created by Example) in APP (created by Example) in Example ``` The function gets passed to Area.Group as a child: ```jsx Area.Group = class AreaGroup extends AreaComponent { constructor(root, props) { super(root, props); this.children = []; } appendChild(child) { this.children.push(child); } draw(area, p, props) { if(typeof this.props.children === "function"){ this.appendChild(this.props.children(p.getAreaWidth(), p.getAreaHeight())); } for (let i = 0; i < this.children.length; i += 1) { if (typeof this.children[i] === 'object') { console.log(this.children[i]); this.children[i].render(this, area, p, props); } } } }; ``` The react element is printed to the console (returned by React.createElement via the function child): ```js { '$$typeof': Symbol(react.element), type: 'AREATEXT', key: null, ref: null, props: { children: '560 460' }, _owner: null, _store: {} } ``` But I need something like (the real class instance) this for the rendering: ```js AreaText { root: Root { children: [ [App] ] }, props: { children: 'Some text', x: 0, y: 0 }, element: {}, children: [ 'Some Text' ], str: AttributedString {} } ```
Author
Owner

@mischnic commented on GitHub (Jul 31, 2018):

But I need something like (the real class instance) this for the rendering:

In other words: How can I invoke the reconciliation "manually" without rendering these elements (usually you just return them in a render function and the reconciler continues down)? If that's not easily possible the whole Area code would have to be restructured (if possible - probably using Context).

<!-- gh-comment-id:409345022 --> @mischnic commented on GitHub (Jul 31, 2018): > But I need something like (the real class instance) this for the rendering: In other words: How can I invoke the reconciliation "manually" without `render`ing these elements (usually you just return them in a `render` function and the reconciler continues down)? If that's not easily possible the whole Area code would have to be restructured (if possible - probably using Context).
Author
Owner

@kusti8 commented on GitHub (Aug 12, 2018):

I'm actually not sure. Could you upload your code and test so I can try it out? There may be some functions in the reconciler that can help out, like DesktopRenderer.updateContainer(element, node, null); (from render/index.js), but I'm not sure.

<!-- gh-comment-id:412345019 --> @kusti8 commented on GitHub (Aug 12, 2018): I'm actually not sure. Could you upload your code and test so I can try it out? There may be some functions in the reconciler that can help out, like `DesktopRenderer.updateContainer(element, node, null);` (from render/index.js), but I'm not sure.
Author
Owner

@mischnic commented on GitHub (Aug 16, 2018):

diff --git a/examples/area-size.js b/examples/area-size.js
new file mode 100644
index 0000000..fb07aa5
--- /dev/null
+++ b/examples/area-size.js
@@ -0,0 +1,22 @@
+import React, { Component } from 'react';
+import { render, Window, App, Box, Menu, Area } from '../src/';
+
+class Example extends Component {
+  render() {
+    return (
+      <App>
+        <Window title="Test" size={{ w: 600, h: 500 }} margined={true}>
+          <Box padded>
+            <Area>
+              {(width, height) => (
+                <Area.Text>{`${width} ${height}`}</Area.Text>
+              )}
+            </Area>
+          </Box>
+        </Window>
+      </App>
+    );
+  }
+}
+
+render(<Example />);
diff --git a/src/components/Area.js b/src/components/Area.js
index fcc911e..73855d0 100644
--- a/src/components/Area.js
+++ b/src/components/Area.js
@@ -577,9 +577,33 @@ Area.Group = class AreaGroup extends AreaComponent {
   }
 
   draw(area, p, props) {
-    for (let i = 0; i < this.children.length; i += 1) {
-      if (typeof this.children[i] === 'object') {
-        this.children[i].render(this, area, p, props);
+    if (typeof this.props.children === 'function') {
+      /*
+        OUTPUT:
+        
+        Warning: Functions are not valid as a React child. This may happen if you return a Component instead of <Component /> from render. Or maybe you meant to call this function rather than return it.
+            in AREAGROUP (created by Area)
+            in AREA (created by Area)
+            in Area (created by Example)
+            in VERTICALBOX (created by Box)
+            in Box (created by Example)
+            in WINDOW (created by Example)
+            in APP (created by Example)
+            in Example
+        { '$$typeof': Symbol(react.element),
+          type: 'AREATEXT',
+          key: null,
+          ref: null,
+          props: { children: '560 460' },
+          _owner: null,
+          _store: {} }
+      */
+      console.log(this.props.children(p.getAreaWidth(), p.getAreaHeight()));
+    } else {
+      for (let i = 0; i < this.children.length; i += 1) {
+        if (typeof this.children[i] === 'object') {
+          this.children[i].render(this, area, p, props);
+        }
       }
     }
   }

<!-- gh-comment-id:413498906 --> @mischnic commented on GitHub (Aug 16, 2018): ```diff diff --git a/examples/area-size.js b/examples/area-size.js new file mode 100644 index 0000000..fb07aa5 --- /dev/null +++ b/examples/area-size.js @@ -0,0 +1,22 @@ +import React, { Component } from 'react'; +import { render, Window, App, Box, Menu, Area } from '../src/'; + +class Example extends Component { + render() { + return ( + <App> + <Window title="Test" size={{ w: 600, h: 500 }} margined={true}> + <Box padded> + <Area> + {(width, height) => ( + <Area.Text>{`${width} ${height}`}</Area.Text> + )} + </Area> + </Box> + </Window> + </App> + ); + } +} + +render(<Example />); diff --git a/src/components/Area.js b/src/components/Area.js index fcc911e..73855d0 100644 --- a/src/components/Area.js +++ b/src/components/Area.js @@ -577,9 +577,33 @@ Area.Group = class AreaGroup extends AreaComponent { } draw(area, p, props) { - for (let i = 0; i < this.children.length; i += 1) { - if (typeof this.children[i] === 'object') { - this.children[i].render(this, area, p, props); + if (typeof this.props.children === 'function') { + /* + OUTPUT: + + Warning: Functions are not valid as a React child. This may happen if you return a Component instead of <Component /> from render. Or maybe you meant to call this function rather than return it. + in AREAGROUP (created by Area) + in AREA (created by Area) + in Area (created by Example) + in VERTICALBOX (created by Box) + in Box (created by Example) + in WINDOW (created by Example) + in APP (created by Example) + in Example + { '$$typeof': Symbol(react.element), + type: 'AREATEXT', + key: null, + ref: null, + props: { children: '560 460' }, + _owner: null, + _store: {} } + */ + console.log(this.props.children(p.getAreaWidth(), p.getAreaHeight())); + } else { + for (let i = 0; i < this.children.length; i += 1) { + if (typeof this.children[i] === 'object') { + this.children[i].render(this, area, p, props); + } } } } ```
Author
Owner

@maciek134 commented on GitHub (Sep 1, 2018):

Functions as React children is a bad idea. Why not use Refs? Just allow the pixel size to be queried using getWidth/getHeight, then you can do

  constructor(props) {
    super(props);
    this.area = React.createRef();
  }
  
  render() {
    return <Area ref={this.area}>
      <Area.Text>{this.area.current.getWidth()}px</Area.Text>
    </Area>;
  }

I'm actually working on something like that for a code editor I'm making, I'll make a PR when it works.

<!-- gh-comment-id:417848350 --> @maciek134 commented on GitHub (Sep 1, 2018): Functions as React children is a bad idea. Why not use [Refs](https://reactjs.org/docs/refs-and-the-dom.html)? Just allow the pixel size to be queried using `getWidth/getHeight`, then you can do ```js constructor(props) { super(props); this.area = React.createRef(); } render() { return <Area ref={this.area}> <Area.Text>{this.area.current.getWidth()}px</Area.Text> </Area>; } ``` I'm actually working on something like that for a code editor I'm making, I'll make a PR when it works.
Author
Owner

@maciek134 commented on GitHub (Sep 1, 2018):

@mischnic also about the gradients. I think the first option is more in line with how it's done in React / RN, for example with Stylesheets:

const styles = StyleSheet.create({
  container: {
    borderRadius: 4,
    borderWidth: 0.5,
    borderColor: '#d6d7da',
  },
  title: {
    fontSize: 19,
    fontWeight: 'bold',
  },
  activeTitle: {
    color: 'red',
  },
});
<View style={styles.container}>
  <Text style={[styles.title, this.props.isActive && styles.activeTitle]} />
</View>

Plus it's not a good idea to carry around the overhead of React nodes when all you want is the gradient object (and you are one step from defining the gradient in XML).

So maybe something like

const gradient = Gradient.create({
  type: 'linear',
  x1, y1,
  x2, y2,
  stops: {
    0.0: 'red',
    1.0: 'blue',
  },
});

or

const gradient = Gradient.create('linear', x1, y1, x2, y2, {
  0.0: 'red',
  1.0: 'blue',
});
<!-- gh-comment-id:417855698 --> @maciek134 commented on GitHub (Sep 1, 2018): @mischnic also about the gradients. I think the first option is more in line with how it's done in React / RN, for example with [Stylesheets](https://facebook.github.io/react-native/docs/stylesheet): ```js const styles = StyleSheet.create({ container: { borderRadius: 4, borderWidth: 0.5, borderColor: '#d6d7da', }, title: { fontSize: 19, fontWeight: 'bold', }, activeTitle: { color: 'red', }, }); ``` ```js <View style={styles.container}> <Text style={[styles.title, this.props.isActive && styles.activeTitle]} /> </View> ``` Plus it's not a good idea to carry around the overhead of React nodes when all you want is the gradient object (and you are one step from defining the gradient in XML). So maybe something like ```js const gradient = Gradient.create({ type: 'linear', x1, y1, x2, y2, stops: { 0.0: 'red', 1.0: 'blue', }, }); ``` or ```js const gradient = Gradient.create('linear', x1, y1, x2, y2, { 0.0: 'red', 1.0: 'blue', }); ```
Author
Owner

@mischnic commented on GitHub (Sep 1, 2018):

also about the gradients

These are already implemented in my PR, so I'm quite reluctant to redo them.

I think the first option is more in line with how it's done in React / RN, for example with Stylesheets:

There is nothing like that in "vanilla" React and React Native uses it because it has to serialize everything to json before sending it to the native code side. Creating explicit stylesheets enables them to use a single number to refer to all styles in that stylesheet. We don't need to do that.

Plus it's not a good idea to carry around the overhead of React nodes when all you want is the gradient object.

No React nodes are created, this only creates elements:

{  type: AreaGradient
  key: null,
  ref: null,
  props: { type: "linear", children: [/*same for children*/] },
  _owner: null,
  _store: {} }
<!-- gh-comment-id:417866339 --> @mischnic commented on GitHub (Sep 1, 2018): > also about the gradients These are already implemented in my PR, so I'm quite reluctant to redo them. > I think the first option is more in line with how it's done in React / RN, for example with Stylesheets: There is nothing like that in "vanilla" React and React Native uses it because it has to serialize everything to json before sending it to the native code side. Creating explicit stylesheets enables them to use a single number to refer to all styles in that stylesheet. We don't need to do that. > Plus it's not a good idea to carry around the overhead of React nodes when all you want is the gradient object. No React nodes are created, this only creates elements: ```js { type: AreaGradient key: null, ref: null, props: { type: "linear", children: [/*same for children*/] }, _owner: null, _store: {} } ```
Author
Owner

@maciek134 commented on GitHub (Sep 1, 2018):

No React nodes are created, this only creates elements

My mistake, I didn't look at the code. Still, that's quite a lot of overhead and it's unnecessarily complex. We are writing Javascript, not XML ;p There is no added value in using JSX there.

Simple comparison:

const gradient = <Area.Gradient x1="0" x2="0" x2="100" y2="100">
  <Area.Gradient.Stop color="red" offset="0%"/>
  <Area.Gradient.Stop color="blue" offset="100%"/>
</Area.Gradient>;

vs

const gradient = Gradient.create('linear', 0, 0, 100, 100, {
  0.0: 'red',
  1.0: 'blue',
});
<!-- gh-comment-id:417875893 --> @maciek134 commented on GitHub (Sep 1, 2018): > No React nodes are created, this only creates elements My mistake, I didn't look at the code. Still, that's quite a lot of overhead and it's unnecessarily complex. We are writing Javascript, not XML ;p There is no added value in using JSX there. Simple comparison: ```js const gradient = <Area.Gradient x1="0" x2="0" x2="100" y2="100"> <Area.Gradient.Stop color="red" offset="0%"/> <Area.Gradient.Stop color="blue" offset="100%"/> </Area.Gradient>; ``` vs ```js const gradient = Gradient.create('linear', 0, 0, 100, 100, { 0.0: 'red', 1.0: 'blue', }); ```
Author
Owner

@mischnic commented on GitHub (Sep 1, 2018):

Still, that's quite a lot of overhead

Transforming JSX to React.createElement is done by babel.
React.createElement does basically this (without ref and key handling):

React.createElement = function(type, props, children) {
	return {
		type,
		props: [...props, children]
	}
}

The code for parsing would essentially be the same as this suggestion

const gradient = Gradient.create({
  type: 'linear',
  x1, y1,
  x2, y2,
  stops: {
    0.0: 'red',
    1.0: 'blue',
  },
});

The only argument would correspond to props and stops would be props.children.

We are writing Javascript, not XML.

XML -> HTML -> JSX
Do use React like this ?

import {render, App, Window, Area} from 'proton-native';
import React from 'react';

class MyApp extends React.Component {
	render() {
		return React.createElement(App, null,
			React.createElement(Window, {size: {w: 500, h: 200}, margined: true, title: 'Test'},
				React.createElement(Area, null, [
					React.createElement(Area.Circle, {x: 20, y: 40, r: 10}, null)
				]))
			)
		);
	}
}

render(React.createElement(MyApp, null, null));
<!-- gh-comment-id:417876905 --> @mischnic commented on GitHub (Sep 1, 2018): > Still, that's quite a lot of overhead Transforming JSX to React.createElement is done by babel. React.createElement does basically this (without ref and key handling): ```js React.createElement = function(type, props, children) { return { type, props: [...props, children] } } ``` The code for parsing would essentially be the same as this suggestion ```js const gradient = Gradient.create({ type: 'linear', x1, y1, x2, y2, stops: { 0.0: 'red', 1.0: 'blue', }, }); ``` The only argument would correspond to `props` and `stops` would be `props.children`. > We are writing Javascript, not XML. XML -> HTML -> JSX Do use React like this ? ```jsx import {render, App, Window, Area} from 'proton-native'; import React from 'react'; class MyApp extends React.Component { render() { return React.createElement(App, null, React.createElement(Window, {size: {w: 500, h: 200}, margined: true, title: 'Test'}, React.createElement(Area, null, [ React.createElement(Area.Circle, {x: 20, y: 40, r: 10}, null) ])) ) ); } } render(React.createElement(MyApp, null, null)); ```
Author
Owner

@maciek134 commented on GitHub (Sep 1, 2018):

Ok, this discussion is pointless if you can't see the obvious. JSX has a purpose and it's not replacing JS/JSON. And no, the code for parsing these would not be the same. There wouldn't be n+1 (n being the number of steps) calls to React.createElement for every gradient, 50 pointless lines defining propTypes, another 60 parsing and checking the props and brush recreation every draw call (keep in mind, this means creating a new native object every draw call!).

class Gradient {
  static create(options) {
    const brush = new libui.DrawBrush();
    brush.type = options.type;
    brush.start = new libui.Point(options.x, options.y);
    brush.end = new libui.Point(options.width || options.x, options.height || options.y); // optional for radial gradients
    brush.outerRadius = options.r;
    brush.stops = Object.entries(options.stops)
      .map(([stop, color]) => new libui.BrushGradientStop(stop, toLibuiColor(Color(color)));
  }

  static createLinear(x, y, width, height, stops = {}) {
    return Gradient.create({
      type: libui.brushType.linearGradient,
      x, y,
      width, height,
      stops,
    });
  }
  
  static createRadial(x, y, r, stops = {}) {
    return Gradient.create({
      type: libui.brushType.radialGradient,
      x, y, r,
      stops,
    });
  }
};

Not even 30 lines. Usage:

const gradient1 = Gradient.createLinear(0, 0, 100, 100, {
  0.0: 'blue',
  1.0: 'red',
});
const gradient2 = Gradient.createRadial(0, 0, 100, {
  0.0: 'red',
  1.0: 'orange',
});
<!-- gh-comment-id:417884818 --> @maciek134 commented on GitHub (Sep 1, 2018): Ok, this discussion is pointless if you can't see the obvious. JSX has a purpose and it's not replacing JS/JSON. And no, the code for parsing these would not be the same. There wouldn't be `n+1` (`n` being the number of steps) calls to `React.createElement` for every gradient, 50 pointless lines defining propTypes, another 60 parsing and checking the props and brush recreation every draw call (keep in mind, this means creating a new native object every draw call!). ```js class Gradient { static create(options) { const brush = new libui.DrawBrush(); brush.type = options.type; brush.start = new libui.Point(options.x, options.y); brush.end = new libui.Point(options.width || options.x, options.height || options.y); // optional for radial gradients brush.outerRadius = options.r; brush.stops = Object.entries(options.stops) .map(([stop, color]) => new libui.BrushGradientStop(stop, toLibuiColor(Color(color))); } static createLinear(x, y, width, height, stops = {}) { return Gradient.create({ type: libui.brushType.linearGradient, x, y, width, height, stops, }); } static createRadial(x, y, r, stops = {}) { return Gradient.create({ type: libui.brushType.radialGradient, x, y, r, stops, }); } }; ``` Not even 30 lines. Usage: ```js const gradient1 = Gradient.createLinear(0, 0, 100, 100, { 0.0: 'blue', 1.0: 'red', }); const gradient2 = Gradient.createRadial(0, 0, 100, { 0.0: 'red', 1.0: 'orange', }); ```
Author
Owner

@mischnic commented on GitHub (Sep 1, 2018):

every draw call

Thats a good point. But it can be easily solved using memoization with a WeakMap. (With a gradient based on e.g. the mouse position, your solution would suffer from the same problem.)

The question is, which API is better (ignoring the underlying implementation)? @kusti8

<!-- gh-comment-id:417886231 --> @mischnic commented on GitHub (Sep 1, 2018): > every draw call Thats a good point. But it can be easily solved using memoization with a WeakMap. (With a gradient based on e.g. the mouse position, your solution would suffer from the same problem.) The question is, which API is better (ignoring the underlying implementation)? @kusti8
Author
Owner

@maciek134 commented on GitHub (Sep 1, 2018):

No, because with my approach you can access libui.DrawBrush directly and just change what's needed. Still only one object per gradient. Memoizing mouse position based gradients would eat the RAM faster than Chrome.

<!-- gh-comment-id:417886567 --> @maciek134 commented on GitHub (Sep 1, 2018): No, because with my approach you can access `libui.DrawBrush` directly and just change what's needed. Still only one object per gradient. Memoizing mouse position based gradients would eat the RAM faster than Chrome.
Author
Owner

@mischnic commented on GitHub (Sep 1, 2018):

No, because with my approach you can access libui.DrawBrush directly and just change what's needed. Still only one object per gradient.

How low level do you want to go? You could ditch the whole state concept and just set the text of a label using myLabel.text = "...";.

<!-- gh-comment-id:417887591 --> @mischnic commented on GitHub (Sep 1, 2018): > No, because with my approach you can access libui.DrawBrush directly and just change what's needed. Still only one object per gradient. How low level do you want to go? You could ditch the whole state concept and just set the text of a label using `myLabel.text = "...";`.
Author
Owner

@maciek134 commented on GitHub (Sep 2, 2018):

It's not low level, if I wanted low level I'd just export the libui.DrawBrush constructor and be done with it. Your approach is complex for the sake of being complex and has so much drawbacks I'm surprised you went with it. @kusti8 I think you'll have to make a decision here 😜

<!-- gh-comment-id:417909215 --> @maciek134 commented on GitHub (Sep 2, 2018): It's not low level, if I wanted low level I'd just export the `libui.DrawBrush` constructor and be done with it. Your approach is complex for the sake of being complex and has so much drawbacks I'm surprised you went with it. @kusti8 I think you'll have to make a decision here 😜
Author
Owner

@kusti8 commented on GitHub (Sep 4, 2018):

It's a tough decision. I'm leaning towards the JSX approach simply because if we end up making something like a stylesheet, that could lead to it ending up being cluttered, so that we have 3-5 StyleSheet type objects. With react, gradients end up being in the stylesheet, and I would be fine with it if we ended up making a Stylesheet and adding it there, but I don't really like creating a separate clone of it.

<!-- gh-comment-id:418489617 --> @kusti8 commented on GitHub (Sep 4, 2018): It's a tough decision. I'm leaning towards the JSX approach simply because if we end up making something like a stylesheet, that could lead to it ending up being cluttered, so that we have 3-5 StyleSheet type objects. With react, gradients end up being in the stylesheet, and I would be fine with it if we ended up making a Stylesheet and adding it there, but I don't really like creating a separate clone of it.
Author
Owner

@maciek134 commented on GitHub (Sep 8, 2018):

Please consider the underlying implementation when making this decision. The JSX one is complex, resource-heavy and confusing - nowhere in the React ecosystem you define things with JSX, it's only used for rendering components.

If you knew nothing about this library and looked at the gradient code, wouldn't you think that it's a component of some kind?

I think the question we should ask when deciding whether to use JSX for something is:

Is this code normally written inside of the render() method, or outside of it?

<!-- gh-comment-id:419672212 --> @maciek134 commented on GitHub (Sep 8, 2018): Please consider the underlying implementation when making this decision. The JSX one is complex, resource-heavy and confusing - nowhere in the React ecosystem you define things with JSX, it's only used for rendering components. If you knew nothing about this library and looked at the gradient code, wouldn't you think that it's a component of some kind? I think the question we should ask when deciding whether to use JSX for something is: > Is this code normally written inside of the `render()` method, or outside of it?
Author
Owner

@mischnic commented on GitHub (Sep 10, 2018):

If you knew nothing about this library and looked at the gradient code, wouldn't you think that it's a component of some kind?

Is this code normally written inside of the render() method, or outside of it?

Thats a good point...

so that we have 3-5 StyleSheet type objects

I don't see anything upcoming in libui that would need a stylesheet.

<!-- gh-comment-id:419835032 --> @mischnic commented on GitHub (Sep 10, 2018): >If you knew nothing about this library and looked at the gradient code, wouldn't you think that it's a component of some kind? >Is this code normally written inside of the render() method, or outside of it? Thats a good point... > so that we have 3-5 StyleSheet type objects I don't see anything upcoming in libui that would need a stylesheet.
Author
Owner

@maciek134 commented on GitHub (Sep 10, 2018):

Yup, proton-native shouldn't need stylesheets at all.

<!-- gh-comment-id:419845008 --> @maciek134 commented on GitHub (Sep 10, 2018): Yup, `proton-native` shouldn't need stylesheets at all.
Author
Owner

@kusti8 commented on GitHub (Sep 17, 2018):

Sorry about the delay.

That's a good point. Libui won't be getting stylesheets, was thinking a bit in the future about that.

@maciek134 I like your second syntax here, although it is hard to understand what each number is from a first glance. I'm thinking a more CSS like thing where it accepts an object, so {x1: 0, y1: 0 ...}, but it may not be needed if its documented.

const gradient1 = Gradient.createLinear(0, 0, 100, 100, {
  0.0: 'blue',
  1.0: 'red',
});
const gradient2 = Gradient.createRadial(0, 0, 100, {
  0.0: 'red',
  1.0: 'orange',
});
<!-- gh-comment-id:422185404 --> @kusti8 commented on GitHub (Sep 17, 2018): Sorry about the delay. That's a good point. Libui won't be getting stylesheets, was thinking a bit in the future about that. @maciek134 I like your second syntax here, although it is hard to understand what each number is from a first glance. I'm thinking a more CSS like thing where it accepts an object, so `{x1: 0, y1: 0 ...}`, but it may not be needed if its documented. ``` const gradient1 = Gradient.createLinear(0, 0, 100, 100, { 0.0: 'blue', 1.0: 'red', }); const gradient2 = Gradient.createRadial(0, 0, 100, { 0.0: 'red', 1.0: 'orange', }); ```
Author
Owner

@maciek134 commented on GitHub (Sep 18, 2018):

Well, that's how it's done in JS' Canvas: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/createLinearGradient

CanvasGradient ctx.createLinearGradient(x0, y0, x1, y1);

And let's be honest, even vim has code completion/hints/IntelliSense today, no need to be overly expressive with the parameters.

<!-- gh-comment-id:422512135 --> @maciek134 commented on GitHub (Sep 18, 2018): Well, that's how it's done in JS' Canvas: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/createLinearGradient ``` CanvasGradient ctx.createLinearGradient(x0, y0, x1, y1); ``` And let's be honest, even `vim` has code completion/hints/IntelliSense today, no need to be overly expressive with the parameters.
Author
Owner

@mischnic commented on GitHub (Sep 22, 2018):

And let's be honest, even vim has code completion/hints/IntelliSense today, no need to be overly expressive with the parameters.

How should completions know about that proton-native function? You would need something like typescript definitions.

<!-- gh-comment-id:423731213 --> @mischnic commented on GitHub (Sep 22, 2018): > And let's be honest, even vim has code completion/hints/IntelliSense today, no need to be overly expressive with the parameters. How should completions know about that proton-native function? You would need something like typescript definitions.
Author
Owner

@maciek134 commented on GitHub (Sep 22, 2018):

No, not really. Most of the autocompletion solutions easily parse JS libraries, but it doesn't matter in this case since there are TS definitions for proton-native already: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/proton-native/index.d.ts

<!-- gh-comment-id:423738224 --> @maciek134 commented on GitHub (Sep 22, 2018): No, not really. Most of the autocompletion solutions easily parse JS libraries, but it doesn't matter in this case since there are TS definitions for proton-native already: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/proton-native/index.d.ts
Author
Owner

@kusti8 commented on GitHub (Sep 25, 2018):

If it's already used with JS canvas then it would make sense to do it that way. I'm still not a fan of the entire approach, I just like more verbose functions than less, but if it's standard then it's fine.

<!-- gh-comment-id:424494530 --> @kusti8 commented on GitHub (Sep 25, 2018): If it's already used with JS canvas then it would make sense to do it that way. I'm still not a fan of the entire approach, I just like more verbose functions than less, but if it's standard then it's fine.
Author
Owner

@mischnic commented on GitHub (Nov 1, 2018):

Example of a radial gradient with different outer circle point, maany parameters:

const radialGradient2 = Area.Gradient.createRadial(200, 50, 230, 50, 10, {
  0: 'orange',
  1: 'blue',
});
<!-- gh-comment-id:435138521 --> @mischnic commented on GitHub (Nov 1, 2018): Example of a radial gradient with different outer circle point, maany parameters: ```js const radialGradient2 = Area.Gradient.createRadial(200, 50, 230, 50, 10, { 0: 'orange', 1: 'blue', }); ```
Author
Owner

@kusti8 commented on GitHub (Nov 3, 2018):

Yeah it's long, but if it matches web JS then I prefer that over simplifying it.

<!-- gh-comment-id:435617308 --> @kusti8 commented on GitHub (Nov 3, 2018): Yeah it's long, but if it matches web JS then I prefer that over simplifying it.
Author
Owner

@mischnic commented on GitHub (Nov 3, 2018):

Fine, but I don't think many know the function signature of createRadialGradient(x1,y1,r1,x2,y2,r2).

<!-- gh-comment-id:435617424 --> @mischnic commented on GitHub (Nov 3, 2018): Fine, but I don't think many know the function signature of `createRadialGradient(x1,y1,r1,x2,y2,r2)`.
Author
Owner

@kusti8 commented on GitHub (Nov 9, 2018):

Even if they don't know it and however much I don't like it myself, it is already established and I'd rather have something that mirrors what is established rather than something new. I would have made proton-native a new target for react-native instead of with a new reconciler, but a lot of features like flexbox View weren't available.

<!-- gh-comment-id:437219394 --> @kusti8 commented on GitHub (Nov 9, 2018): Even if they don't know it and however much I don't like it myself, it is already established and I'd rather have something that mirrors what is established rather than something new. I would have made proton-native a new target for react-native instead of with a new reconciler, but a lot of features like flexbox View weren't available.
Author
Owner

@kusti8 commented on GitHub (Jan 19, 2020):

Proton Native V2 is now released! If the issue still occurs in the new update, please open a new issue.

<!-- gh-comment-id:576029518 --> @kusti8 commented on GitHub (Jan 19, 2020): Proton Native V2 is now released! If the issue still occurs in the new update, please open a new issue.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/proton-native#76
No description provided.