-
-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: API - Move from 'style' to Compositional Components #489
Comments
One other motivation we had for our current |
@bryphe to break it down; I like the approach of components for layout or gestures. However, some of them are semantically unclear. Things become tricky when we have a component parent and multiple children (like:
When it comes to other use cases, I think it's a great idea. I outlined the changes needed to refactor the reconciler to support nesting different instances of "React" here |
Thanks for the feedback @wokalski , great points.
Good point, this is tricky! Flutter solves it by making their Transform components only work with a single child. We could do something similar; we just don't have a good way to express this with the type system today (it'd have to be a runtime error). Alternatively, we could treat a list of children as sort of a virtual container. This is important for transforms like rotation, where the default expected behavior is to rotate around the 'center' - but where is the 'center' for a group? We could create a center based on an aggregated axis-aligned bounding box of the components, and that might be OK - but not 100% sure.
I'm not sure what this part means - transforms would be applied up the chain (every 'component' has a transform, even though it might be an identity transform). Essentially during rendering we traverse the node-tree and apply the transforms via matrix multiplication across the train.
Good point again! I think this is a similar trade-off with the transform. In flutter, I guess they actually don't have these widgets - it's a But I think we could have two options again:
In Option 1, we'd throw a runtime error. In Option 2, we'd figure out the dimensions of each container (dependent on the layout strategy), create an aggregate axis-aligned bounding box, and then during paint, we'd render the box shadow according to the aggregate. If it's a single item - the bounding box would be the single item's bounding box. I think, without having the type system be able to validate at compile-time that the element only has a single child, it'd be preferable to implement Option 2 (although that incurs more implementation complexity). Interested in thoughts though! |
I like the idea of having styles more declarative and most of the time we abstract things away like this during my day-to-day using React, either by making or own abstractions and/or with the help of something like: https://styled-system.com/api/. But I wonder if it'd be hard to cover every corner-case which can be solved with a One question that comes up is how to handle pseudo/state-styles? Would every Primitive require an extra set of properties for e.g. <Container
activeColor=Colors.green
color=Colors.red
hoverColor=Colors.blue
>
...
</Container> Nonetheless it's a cool direction to explore and points to one of the big benefits when using Flutter which enables you to move very fast using their provided UI-abstractions! Following! 🙂 |
The multiple children problem is a real one - one we should definitely think through thoroughly. Concerning option 1, only allowing a single child: I'm not sure if that would be a desirable or understandable solution. My current idea of JSX and html semantics is that you can always put multiple children inside one element and it should be clear what happens with that. There are a few cases where only certain element types are allowed, but I would even consider those bad design decisions. I don't know of any existing limitations to one element anywhere though. So I think we should avoid this solution. About option two: making these primitives implicit containers with respect to some semantics What I'm getting at with this is a third idea: This would be a middle ground between the fine grained widgets from flutter and the one stop shop style prop from css. I think it would be the best abstraction for the JSX world. About the hover, active, disabled etc states - I'm unsure if we should stick that closely to web concepts here. In react, if this weren't part of the DOM, one would typically model these using a custom |
briskml/brisk-reconciler#46 will create some really good facilities to implement it in a type safe manner. |
I really like the idea of layout components, but still a bit uncertain about "decoration" components. I'd love to play around with a prototype though. Re: multiple children for BoxShadow Restricting to a single child with a runtime error seems less than great (for obvious reasons I hope). It could perhaps be a warning though, if the behavior in the case of multiple children is either degraded or unreasonably expensive. Option 2, the bounding box, also doesn't seem all that reasonable to me, assuming the bounding box is rectangular, as "boxes" typically are. It seems somewhat reasonable for a border, but I would expect a However, even in the case of a single child, it should use the clip are, not the bounding box. And even for borders, it should be possible to create rounded and even circular/elliptical borders. If not using the clip area for this it certainly needs to add to the clip area. Does it matter whether there's one or more children, then, if it's based on clip area? (Implicit) Inheritance of "style" properties It would be very convenient to have certain properties inherited, the font properties in particular. Otherwise, in order to consistently render text, you ought to pass around all these properties: Additionally, in design work, you usually don't work with absolutes. Like with layout, which you specify relative to the current container, not to the whole. You say "this container should be split into three equal parts", not "this container should have three parts that are a fifth the size of the window it's in". The same applies to many style properties. Most trivially, line-height is of course typically based on the size of the text that should occupy the lines, but also paddings and margins, border widths and even the size of adjacent elements need to be sized relative to each other. That means they need some common reference, which is usually the font size. |
This is a very relevant summary of the issues we currently have within react itself around element composition and interaction between the elements. |
Any more findings here? 🙂 I think trying to settle on a form would be a big step forward in opening up for a lot of other UI-work such as revamping and creating base components among other things! As @glennsl was saying I think the biggest issue with the current compositional components is that they don't let its style props through. Like this (IIRC):
(children of Padding are no longer in a row) |
Might be relevant and/or helpful: |
The `opacity` style option was removed in #540 , in trying to factor to semantic components. However, since we don't have that yet (dependent on some larger refactorings, as discussed in #489 ) - we should bring this back. I'd prefer to use a more semantic component like `<Opacity />`, however, in the current design, that can impact layout, as every primitive has an associated layout node.
This came from some discussions with @manuhornung and others around Flutter's model vs our
style
model. IMO, one thing Flutter did really well in their design is their compositional component model (moving away from the uber-style
grab bag of properties).To clarify, what I mean by a compositional model, is instead of having a
Style.[...]
with arbitrary properties (like positioning, transform, etc), Flutter has more scoped widgets - like aTransform
widget, orPositioned
widget. This is nice in terms of discoverability - each of those widgets takes a relatively small and concise set of properties.In addition, the
Style.[..]
model is coupled to a single layout system, whereas Flutter uses component composition to express a variety of layout models - this is very convenient for building UI apps.The
Style.[..]
model has gotten us really far, but there are a few downsides I've seen so far:Style.[..]
.fold
our style-list into a style-object:revery/src/UI/Style.re
Line 501 in 0eb56f4
revery/src/UI/Style.re
Line 589 in 0eb56f4
Node
class ends up having to do a lot of different things - recompute transforms, even if there is are no extra transforms, check the overflow property, etc.I think the compositional model could help with some of the discoverability, by having a more constrained set of properties per-component, and help with the performance by giving each component a more constrained set of responsibilities (potentially minimizing extra transform calculations, overflow calculations, etc).
If people are on-board with moving to the compositional model vs the uber-style model, I'd propose tackling it incrementally, by removing style properties and replacing them with compositional components:
transform
property could become the responsibility ofTransform.Rotate
/Transform.Scale
, etc components ([WIP] API: Transform composite component #488)position
andtop
/left
/right
/bottom
could become the responsibility ofStack
andPositioned
components (Feature - API: Some initial compositional components #483)backgroundColor
property could become thecolor
property of aContainer
color
property could become thecolor
property of aText
padding
property could be moved to aPadding
componentRow
andColumn
componentsborder
properties could be moved to aBorder
componentopacity
could be moved to anOpacity
componentoverflow
could be moved to aClipRect
componentboxShadow
could be moved to aBoxShadow
componentcursor
and mouse event handling could be moved to aMouseInput
componentKeyboardInput
componentIn addition, this model would be amenable for future work:
GestureDetector
component (potentially plugging into @jordwalke 's prototype)The ultimate goal would be to remove the
style
property entirely, and represent everything it modelled via new components, instead.Some things to think about:
Node
in our scene-tree (aka, would they be primitive components?)? If so, they probably don't all need to generateLayoutNode
's - we might need to take a look at that. Also, if we did model these asNode
's - likely a lot of the 'core' behavior in theNode
class could be factored across these new, smaller primitives. For example - thisOverflow.render
behavior would only need to be used by the newClipRect
's node.The text was updated successfully, but these errors were encountered: