-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fix #10601: Added Edit and Reset workspace button #13279
Fix #10601: Added Edit and Reset workspace button #13279
Conversation
dispatcher()->reg(this, "configure-workspaces", this, &WorkspaceActionController::openConfigureWorkspacesDialog); | ||
dispatcher()->reg(this, "reset-workspace", [this]() { setCurrentWorkspaceName(DEFAULT_WORKSPACE_NAME); }); |
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.
As I explained in #12658 (comment), this will not really reset the workspace... not sure what to do about that.
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.
How about basing the default workspace of a "Default workspace template" (not exposed to users). But we can call it the default workspace since for the users its the workspace that they use (by default).
So, whenever we want to reset it to default workspace, we just delete the default workspace and create a new default workspace based off of the template.
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.
Actually 'reset workspace' doesn't work for me in MU3 at all.
So I don't know how it is should work.
In the beginning, I thought that this should reset the current workspace to default state.
But here I see that we are resetting the selection to the default workspace, not the state
<< makeMenuItem("configure-workspaces"); | ||
<< makeMenuItem("edit-workspace") | ||
<< makeMenuItem("configure-workspaces") | ||
<< makeMenuItem("reset-workspace"); |
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.
Maybe it is not a good place for 'edit-workspace' and 'reset-workspace'.
Because these two options affect to current workspace, 'configure-workspaces' affects all workspaces.
@Tantacrul
What do you think about adding these two operations to the Workspaces dialog for each workspace? Like in Parts dialog
I'm unable to test this because the builds have expired. @HemantAntony - can you fix this up and I'll take a look? Thanks! @Eism - I do worry (from what you are saying) that there might be some redundancy here. Need to test it to be sure. |
5968558
to
15862d6
Compare
@Tantacrul It should be okay now |
I agree. @HemantAntony - I don't think this is the right place to put those options because they are hard to find and are much less clear than they would be if they were located in the 'Configure Workspaces' popup. I'd suggest leaving the options in the top menu but also creating some UI options in the Workspace popup itself. Perhaps @bkunda can help with this? |
Actually, I'm confused about what 'Reset Workspace' is supposed to do. In this build, it seems to simply choose the default workspace and that's it. Even if I change the default workspace, pressing 'Reset' doesn't do anything. If this is all it does, then I don't think we should have this option at all. Anyone have any ideas? |
I can tell you what I would want it to, which is not what it has done in the past: I'd want it to reset any customizations I had made during that particular editing session. So if I had a nice carefully customized workspace I had been using for weeks, then one day I accidentally messed it up, I could easily go back to the way it was when I started that session. This would be easy to implement I think since as far as I know we don't actually write the customizations out until your either close the window or change workspaces. So basically it would just amount to reloading the current workspace. What it actually did in MU3 was something rather different, and sort of useful but not really. When you create a workspace, it's cloned from whatever workspace you are in. So you can start form either Basic and add to it, or from Advanced and subtracted from it. Reset takes you back to either Basic or Advanced - whichever your workspace was based on. You can also base one custom workspace off of another, which is nice, but reset still takes you all the way back to Basic or Advanced rather than back to the workspace you actually based yours on. Seems like there was some technical reason for that but I don't remember. |
So, I guess you'd need two options then: Reset workspace: does what it says - resets the selected workspace back to the initial default the app is loaded with. Anything else is just confusing. Undo changes to workspace in this session: again, does what it says, although I'd like to think of a snappier name. Perhaps Undo recent changes? And these options should be clearly applicable to individual workspaces, possibly using a '...' button on each. |
@HemantAntony - if you think you'd be comfortable implementing a design like we've just discussed, I can put it together for you? Apologies for the changes. The MS3 system just feels dislocated and hard to understand. I still don't actually know what it exactly does! |
@Tantacrul That would be great! I'll work on it :) |
@Tantacrul Just pinging you in case you forgot |
Hey @HemantAntony, I wasn't aware I needed to do anything. 😆 I did just try to grab a build but it has expired. Have you implemented changes and want them tested? |
Ah no 😁. You had said that you would implement a design for 'Edit workspace' and 'Undo recent changes' in the above comments |
Sure. I think you're right that it needs a visualisation (and possibly a little double-checking on the UX). I'll bump this over to @bkunda and @jessjwilliamson. They'll likely be able to bang it out quite quickly and give it to you. Right now, I'm a bit too consumed with other things to do the design myself. Apologies! |
I see, no problem! |
fa1f8d3
to
525a11a
Compare
With #10601 supposedly implemented through #26384, can this pull request be closed, @Eism, @cbjeukendrup? |
Yes, we should close this @HemantAntony hey! Thanks for your work! |
@Eism yeah no problem :) |
Resolves: #10601