geido commented on a change in pull request #16394:
URL: https://github.com/apache/superset/pull/16394#discussion_r694792055
##########
File path: superset-frontend/src/components/Modal/Modal.tsx
##########
@@ -46,6 +53,10 @@ export interface ModalProps {
wrapProps?: object;
height?: string;
closable?: boolean;
+ resizable?: boolean;
+ resizableConfig?: ResizableProps;
+ draggable?: boolean;
+ draggableConfig?: DraggableProps;
Review comment:
I would say that `resizableConfig` and `draggableConfig` might be a bit
overkilling here and the only reason why I put them there is that I cannot
predict right now how many of those options we will need in the future. We
might as well realize we don't need any and just shut them down.
So rather than squeezing `resizable` and `draggable` inside the config which
looks like very standard props, I would first implement these types of Modals
in a few places and then either shut the config props down altogether or just
come up with a few elected ones as overridable defaults. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
