mirror of
https://github.com/kusti8/proton-native.git
synced 2026-05-15 14:15:50 -06:00
[GH-ISSUE #130] Missing features in Area #76
Labels
No labels
bug
documentation
enhancement
libui issue
pull-request
question
wait for libui implementation
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: github-starred/proton-native#76
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @mischnic on GitHub (May 16, 2018).
Original GitHub issue: https://github.com/kusti8/proton-native/issues/130
UiArea.beginWindowMove&UiArea.beginWindowResize)@mischnic commented on GitHub (Jul 18, 2018):
@kusti8 How can this be done from a API design perspective? There is no
drawcallback in declarative React code. The only way I can think of is "function as children" ("render callback"):Is that information even needed or is it enough to have
width="100%"?@mischnic commented on GitHub (Jul 18, 2018):
I went with the second option
And regarding gradients: how should they be declared?
1.
@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.
@mischnic commented on GitHub (Jul 22, 2018):
I'm struggling getting that to work.
First, there is always this warning:
The function gets passed to Area.Group as a child:
The react element is printed to the console (returned by React.createElement via the function child):
But I need something like (the real class instance) this for the rendering:
@mischnic commented on GitHub (Jul 31, 2018):
In other words: How can I invoke the reconciliation "manually" without
rendering these elements (usually you just return them in arenderfunction 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).@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.@mischnic commented on GitHub (Aug 16, 2018):
@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 doI'm actually working on something like that for a code editor I'm making, I'll make a PR when it works.
@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:
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
or
@mischnic commented on GitHub (Sep 1, 2018):
These are already implemented in my PR, so I'm quite reluctant to redo them.
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.
No React nodes are created, this only creates elements:
@maciek134 commented on GitHub (Sep 1, 2018):
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:
vs
@mischnic commented on GitHub (Sep 1, 2018):
Transforming JSX to React.createElement is done by babel.
React.createElement does basically this (without ref and key handling):
The code for parsing would essentially be the same as this suggestion
The only argument would correspond to
propsandstopswould beprops.children.XML -> HTML -> JSX
Do use React like this ?
@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(nbeing the number of steps) calls toReact.createElementfor 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!).Not even 30 lines. Usage:
@mischnic commented on GitHub (Sep 1, 2018):
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
@maciek134 commented on GitHub (Sep 1, 2018):
No, because with my approach you can access
libui.DrawBrushdirectly and just change what's needed. Still only one object per gradient. Memoizing mouse position based gradients would eat the RAM faster than Chrome.@mischnic commented on GitHub (Sep 1, 2018):
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 = "...";.@maciek134 commented on GitHub (Sep 2, 2018):
It's not low level, if I wanted low level I'd just export the
libui.DrawBrushconstructor 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 😜@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.
@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:
@mischnic commented on GitHub (Sep 10, 2018):
Thats a good point...
I don't see anything upcoming in libui that would need a stylesheet.
@maciek134 commented on GitHub (Sep 10, 2018):
Yup,
proton-nativeshouldn't need stylesheets at all.@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.@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
And let's be honest, even
vimhas code completion/hints/IntelliSense today, no need to be overly expressive with the parameters.@mischnic commented on GitHub (Sep 22, 2018):
How should completions know about that proton-native function? You would need something like typescript definitions.
@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
@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.
@mischnic commented on GitHub (Nov 1, 2018):
Example of a radial gradient with different outer circle point, maany parameters:
@kusti8 commented on GitHub (Nov 3, 2018):
Yeah it's long, but if it matches web JS then I prefer that over simplifying it.
@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).@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.
@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.