-
Notifications
You must be signed in to change notification settings - Fork 59
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
Implementation to allow right panel's resizing and to go into full-screen mode #24
Conversation
This is a great start! Your hack to trigger the resize event is fine. I'd leave it like that for now with a comment that it's there to get the main canvas to resize. I think the right side bar should resize when you drag it. And then to go "fullscreen", there should be a menubar item. (The menubar will eventually be organized in a hierarchy rather than flat. This fullscreen option will be in the View menu, along with zoom in/zoom out, etc.) When you go into fullscreen mode, only the canvas will be shown (create bar, right panel, and menubar will be hidden). There will be a little button in the upper-right corner that toggles you back out of fullscreen mode. What do you think? Thanks for the work so far. 👍 |
Thanks for the encouragement :) Your suggestions make good sense to me. The button placement for full-screen and edit mode troubles me a bit though. We'd end up having the "full screen" button in the menu, and the button to return to "edit" mode floating in the top right corner... Microsoft OneNote application has a similar functionality, but they put the button in a static location at the top right corner. Push it once => full screen, push it again => edit. This could make make the interface more consistent. Did I manage to make sense with the explication? Let me know if you want a screenshot. |
…to a drag-to-resize one.
Yes I see what you're saying. Sounds good. |
}, | ||
R.div { | ||
className: "RightResize" | ||
ref: "resize" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of giving this a ref in order to make it respond to mousedown (in line 22-23), you can just give this a onMouseDown
prop. That is, change the line to:
onMouseDown: @_onResizeMouseDown
This will call @_onResizeMouseDown with the expected mouseDownEvent
.
OK I pushed everything to my branch. Thanks for the tip on the "onMouseDown" event. I refactored it accordingly. As you can see, in the end I put the full-screen toggle button in the menubar. I got lots of "inspiration" from the Mac OS fullscreen button ;) Please take a look at the whole thing and let me know what you think. As always, recommendations/suggestions are greatly appreciated. |
Looking good. Here are some ideas: It would be nice if there was less layout logic within each of the "section" views (CreatePanel, Canvas, RightPanel, Menubar). If we were to use flexbox to lay out these sections, I think we could make RightPanel resize by adjusting its width, and then have Canvas just take up the remaining space using flexbox, without having to have any layout logic within Similarly, instead of putting a I should let you know: my plan for making diagrams embeddable on web pages is essentially fullscreen mode (no sidebars) where you can only drag shapes that are controllers (the ones that highlight red). So the fullscreen mode that you've put together may evolve into this "preview" mode, where essentially you're seeing/manipulating your diagram as it would appear embedded on an external web page. So thank you for putting this together so far: it is bringing the embedding feature closer, which is very important. For the right sidebar, I would like to remove the extra grey space, the 10 pixel border between the sidebar and the canvas. Perhaps a nice way to do this would just be to make that grey area transparent and overlayed on the canvas, so that the canvas shows through. And then make that transparent area have a |
I'm starting to experiment with the flex layout and I think it's workable. It is a great idea and would help to reduce the complexity of the layout system in general. I'll hack at it for a bit more to see if the whole thing can work. Are you comfortable with this one pull-request that changes the layout system + implement the full-screen mode at the same time? I can open a new pull-request for the layout refactoring if you want. |
The one pull request is fine. Whatever's easiest for you. |
…omponents in full-screen mode.
The new implementation with display: flex is up. Took a bit longer than I expected. Compared to commit 7c1484f, here are the major changes:
This implementation does result in noticeable speed-up when manipulating groups with spread. Canvas zooming speed might be improved as well but it's much less noticeable sur my laptop. This change does impact a lot of the layout css. Please take a careful look and let me know what you think. |
"LayoutMode": true | ||
"FullScreen": layout.fullScreen | ||
} | ||
ref: "LayoutMode" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't need this ref
Getting really close! There's a bug with the create panel thumbnail canvases being resized to the wrong size. Not exactly sure what it's about, but I assume something with the flexbox layout. The icon svg images are referenced now as absolute urls, e.g. Because you use relative resizing (xDelta) to implement the right sidebar resize and because there is a min/max size, if you drag past the bounds and then drag back, the sidebar no longer is "synced" with the mouse. I would suggest this variation on the implementation:
And that's it! These are the only issues I've found. Thanks again, this is great work! |
I fixed the styling issue for the x button and the resizing behavior. The fullscreen and edit buttons are now in the icon font as well. I struggled a bit to find the compilation method for fontcustom. In your readme, you pointed to the npm module fontcustom which didn't seem to do a whole lot. I ended up installing https://github.com/FontCustom/fontcustom/ with homebrew and got it compiled. Is that the way you use it as well? Is it possible to compile with a node module instead? |
I think this looks done! I will merge it. The readme links to fontcustom.com which apparently is down. The link you found is what I was using. I'll update the readme to point at that instead. |
Implementation to allow right panel's resizing and to go into full-screen mode
This is my first implementation for issue #23. It groups the Outline + Inspector into a component called "RightPanel" and adds the capability to close/open the panel by clicking on the vertical bar.
I'm no stranger to web development, but this is literally my first commit with React, Coffeescript and Stylus. It'd be great if you can let me know what you think about it and whether I should pursue this route further.
If the commit is OK with you, I can pursue it further to cleanup/improve the implementation. Some ideas: