-
Notifications
You must be signed in to change notification settings - Fork 487
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
Proxies PanedLayout function calls to extended layouts #1551
base: development
Are you sure you want to change the base?
Conversation
This commit extends CustomLayout class to implemnt PanedLayout, and proxies these functions to the extended layout (if it's PanedLayout as well).
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.
This is cool, but the one point of feedback is that I wonder if the pane layout commands should act as defaults for the custom commands that can be overridden by explicit definitions.
My concern is that if you have multiple custom layouts you may have muscle memory for the custom commands, and I think it would be jarring to have to switch to the other set of commands to get this to work.
@ianyh Thanks for the feedback. Do I interpret your message correctly as meaning: custom command 1 = shrink, custom command 2 = expand, etc.? I understand the point you're making about it being jarring, but I'd like to elaborate on it further. Firstly, the capability to extend built-in or existing layouts is excellent. Amethyst offers a comprehensive set of built-in layouts, so my assumption is that tweaking existing layouts is more common than creating new ones from scratch. However, this conclusion is based on my own perception, so I could be mistaken. In the scenario mentioned above, it seems non-intuitive to, for instance, shrink the main pane using the shortcut designated for a custom command (if the selected custom layout extends a built-in one). For example, in my case, I use a 49" ultra-wide monitor and prefer the 3-column layout. However, when I have only one window open, I want it in the middle column without stretching it to fullscreen. This is easily achievable by extending the built-in 3-column layout and modifying the behavior for that specific case. I might use the two-pane layout if my laptop is not connected to an external monitor. Here's where the non-intuitive aspect comes in: I would then have different key shortcuts for shrinking the pane, depending on the layout in use (the built-in two-pane layout: "shrink main pane," or my custom 3-column layout: "custom command 1"). In my view, I shouldn't need to be aware of the current layout to use the correct shortcut to shrink the pane. Any thoughts on that? In addition, since you know better the overall architecture, could there be any corner cases where PanedLayout is handled differently compared to "normal" layouts? Since my modifications changed |
I've seen a lot of layouts that are made from scratch, but that may be a consequence of not having had extensions at the start. I wonder if the solution here isn't to proxy the commands, but rather to proxy the pane state since that is fairly well defined by the protocol? Or maybe have the option for extensions of pane layouts to proxy to the native commands from a custom command? e.g., {
...
command1: "expandMainPane"
...
} |
As I'm writting a custom layout, which is not extending any layout, I would love to be able to adjust my layout using the native |
Oh yes @garo , that would be great indeed! I have almost the exact same use-case. I'm using 2 custom commands to replicate |
@Henkru Are you able to accept ianyh's change so this can be merged? |
Closes #1550.
This PR extends the Custom Layout class to proxy PanedLayout functions to the extended layout.