improvement: add resizer component #1538
Conversation
|
great! I've configured netlify actions for |
harshithpabbati
force-pushed
the
resize-component
branch
from
eb843f3 to
ddc160e
Compare
Yeah, I saw it. Can you review the PR @acao |
|
@harshithpabbati still reviewing! |
| className={className} | ||
| style={{ | ||
| cursor: `${dir}-resize`, | ||
| ...style, |
There was a problem hiding this comment.
a) should we use style here or _css or sm?
b) would it make more sense to just spread rest props instead of a style prop?
c) should we be using a different component if we want to allow theme-ui props?
There was a problem hiding this comment.
Generally, the style attribute is good to use for dynamic styles, but it doesn't look like that's really the case here, unless dir changes frequently. If dir is changing based on mouse events, etc., then style is fine to use here. For passing through style as props, it's sort of the same thing, but I haven't looked at where that's coming from. Does that make sense?
There was a problem hiding this comment.
yes! ok good, just wanted to make sure we weren't baking in anything that would "break" theme-ui implementations or user extensibility
| const { dir, onStart, onEnd, onUpdate, className, style, children } = props; | ||
|
|
||
| const [isDragging, setIsDragging] = useState(false); | ||
| const listenersRef = useRef<ResizeHandlerData['listenersRef']>(null); |
There was a problem hiding this comment.
would we want to use forwardRef here to allow users to pass ref? or do we only need it internally?
| className={className} | ||
| style={{ | ||
| position: 'relative', | ||
| ...containerStyle, |
There was a problem hiding this comment.
same here, maybe shouldn't spread style props instead of just spreading props
|
@harshithpabbati most of my review questions are related to extensibility. so for now, I think this looks great overall for the firs tpass. as we begin to implement it in our own components, we will probably find ways we need to extend it for our purposes and for the purposes of plugin developers (this includes my comments about forwardRef, etc) |
Yeah sure, we can extend it when we begin implementing that into our components. |
This PR adds a new Resizer component. So we will be able to resize the editors. We can simply wrap the editor with resizer component like this
This above example will add a resizer option below the
maintag and we can also pass the required styles to it.@acao can you review this