-
-
Notifications
You must be signed in to change notification settings - Fork 142
FEATURE: Hide read only workspaces #3011
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
base: 8.4
Are you sure you want to change the base?
FEATURE: Hide read only workspaces #3011
Conversation
This is a bit related to the former issue #2796 |
82fd3d2
to
e3e9c45
Compare
Thanks for the approval, any suggestions on naming things or did I forget anything? |
Thanks @markusguenther for your work. There is one drawback we need to tackle. Scenario:
I can make it work if I rebase the account's user workspace to my read-only workspace after account creation. However, since we don't have a UI for that, this is not feasible. |
Talked here with @lorenzulrich and as the UI uses the configured |
Ok, tried to fix the latest issue @lorenzulrich mentioned and found out that also a regular restricted editors have the issue that the baseWorkspace can be wrong. So the initalState is with the first writeable Workspace, as I adjusted that, but this does not lead tothe fact that the user really has this workspace set as baseWorkspace. The problem here is that I tried to detect that and then just change the baseWorkspace but the initialState is built with a GET Request, and we need a POST to be able to change the Workspace. So I am now at the point where I would say a dialog for the User would be nice. Something like "Your on a non writeable workspace, would you like to change to workspace foo" and when the user confirms that we change the workspace. Or would you say I search for a right place and just do it without confirmation by the user. I mean, in the end, the user cannot do anything when he doesn't switch the workspace. WDYT? |
Ah also played a bit with the WorkspaceHelper and conditionally change the baseWorkspace on editing things. But that feels bit hacky because you need to take care that it is the right place, which is always from a POST. Here was the Idea to adjust the |
I think that a solution with a dialog would be nice. It is more transparent for the user than just switching it on-the-fly. Let's have a look at a real-life example: This is the Neos Documentation. How would the UI react here for me as a restricted user? Would I land in "Next release" without knowing? If so, I don't think that would be a good behaviour. I would then prefer something like: |
Hope to get this polished this evening/night |
I am ill since Yesterday, so i fear that i Will not male it today :/ Sorry... |
Maybe we can get this into 8.1? |
Too late for 8.1 by now, but what a nice feature it would be. 👍 |
Hi @markusguenther is this now ready for a final review? |
I need to finish that ... the idea from @lorenzulrich is not implemented yet. |
uups sorry for closing this accidentally (fat finger on mobil^^) and also sorry for this discussion in another direction: i lately talked with @ahaeslich about this workspace selector and its position in the ui in general. you proposed to cleanup the current implementation with an opt in feature flag i think we should better strive for a nice default than extensive customization. So while the problem will be solved for those knowing the flag my idea is having a workspace select dropdown next to the publish menu (like the user settings select box) if youre wondering why im focusing here on the separately placed dropdown - i just think that otherwise things get too crowded moved to: #3247 |
@markusguenther do you want to finish this for 8.2? |
thanks for explaining that. I hadn’t this in mind but this seems like a valid usecase. I still oppose to the yaml feature flag, but what about a checkbox in the workspace selector someplace to hide and unhinde readonly workspaces (enabled by default) (In case things get too crowded I would prefer the workspace selector in another position (next to the publishing menu) either way) |
bbe0b08
to
75e167a
Compare
75e167a
to
fac37cd
Compare
Will not find the time to finish it this time. So a new feature for 9.0 🙈 |
Seems it needs more love for 9.0 💙 |
When you work with countless backend users and workspaces, it may happen that the restricted editors maybe are confused by all the available workspaces. And sometimes select a workspace that is read only. Then they can not edit content since the latest bug fixes and before that, they run into an error while publishing. This feature should enable the developers and integrators to hide all none writable workspaces in the selection. That should improve the user experience for the editors. By default, the setting is false and therefore everything is like before that feature. When you enable it with the setting initialState.user.settings.hideReadOnlyWorkspaces the user will see only writable workspaces in the publishing dropdown, except there are no writable workspaces. Then we use the default target workspace as fallback. This is often live.
Checks if the baseWorkspace is allowed and change to the first possible permitted workspace if not.
…pace" This reverts commit 182f8d9.
fac37cd
to
0492b3d
Compare
Rebased it to version 8.4 and now just need to finish it 🙈 |
@mhsdesign @Sebobo Short question - how we handle PRs like that. As that would be upmerged to 9.0 it could not work out as 9.0 cannot get a new feature. So the decision to release 8.4 after 9.0 is not the best idea for the upmerge concept. |
Well, when upmerging the question rather is – do we want or need this feature/bugfix. If no, because it does exist already or no longer applies for other reasons, "skip it". If not, adjust it or at least open a ticket to keep a note on "the missing thing" for 9.0. Granted, skipping a feature or bugfix when upmerging might be heard, but it should not be a blocker. If it turns out to be too hard, we can still cherry-pick things and then could even do "an empty merge" to signify things have been taken care of. |
Hm, I assume that projects that go to 8.4 don't upgrade to 9.0. So we could also skip it in the upmerge and add it to 9.1. |
Thanks for the input! Would be great to have the opportunity to introduce features, even if it means being a bit more careful during upmerges. Skipping something can definitely lead to issues if we’re not paying close attention. |
Im against sneaking new features like these or other during an upmerge and have them unintentionally pop up in 9.0.x (some patch). Though we can super simple make an empty merge. That can be done without any issues if 9.0 already contains the latest 8.4 up-merge, then merge this in 8.4 and just discard ALL changes inside the upmerge ... not just the conflicting ones!!!. |
When you work with countless backend users and workspaces, it may happen that the restricted editors maybe are confused by all the available workspaces. And sometimes select a workspace that is read only. Then they can not edit content since the latest bug-fixes and before that, they run into an error while publishing.
This feature should enable the developers and integrators to hide all none writable workspaces in the selection. That should improve the user experience for the editors. By default, the setting is false and therefore everything is like before that feature.
When you enable it with the setting
initialState.user.settings.hideReadOnlyWorkspaces
the user will see only writable workspaces in the publishing dropdown, except there are no writable workspaces. Then we use the default target workspace as fallback. This is often live.Resolves: #3012