Creating a new workspace should also switch to it

Not sure, but I think it's enough if tests pass on 10.1.x to get this in, right?

Yup, but we still need usability review for it :)

Hi,

I'm afraid this feature as it is in patch #9 is definitely a no-go. There a couple of major issues here that fail Nielsen's #1 usability heurestic:
Visibility of system status: "Communicate clearly to users what the system’s state is — no action with consequences to users should be taken without informing them."

1) The same action "Save" should always render the same result: save. If the button does something else, it should be indicated by the button. This relates also to "recognition rather than recall" principle as there is no information given that a switch would happen.

Possible solution: Add another button saying "Save and switch" but keep the option for user to only "Save" as well

2) Saving and switching does not sufficiently inform the user about switching workspace. When user switches a workspace one of the two status messages is displayed (I don't really understand why the difference between the messages for Live and others):

Status message
Third is now the active workspace.

Status message
You are now viewing the live version of the site.

but neither of these messages are displayed when user creates a new workspace while on Live workspace. In that case the message is same with "normal" save action:

Status message
Workspace Fourth has been created.

Possible solution: display both the creation message and the switching message if user clicks "Save and switch".

3) If user switches workspace there is dialogue confirming that user wants to switch to another workspace. There is no confirmation when user saves a new workspace and is switched automatically. This is inconsistent behaviour and especially important as there is no indication in advance that the workspace would be switched. If there is a separate action button for the double action, then this is mitigated.

(Also there is a technical possibility to create workspaces named "Live" where the only change in UI would be the change of background colour in the workspace indicator - but this is not a real problem, only a theoretical case. Could be an April fools joke but not a real use case...)

@simohell, thanks for the review! Implemented all the suggestions from #15 by adding a new Save and activate action button (as the default one) when adding a new workspace, and displaying the activation message separately.

Hiding old patches because we have a MR now.

thanks for implementing the suggestions! i've manually tested, overall it looks good, there is only one small detail i've noticed. The newly added button Save and activate is using the verb "activate" while on the workspaces overview page Switch to... is used on the buttons with "switch" as the verb. Technically if you click the Switch to... button on the overview page you get "Activate the ... workspace" on the confirmation page step. For the Save and activate button that confirmation page step is not necessary and the workspace is directly activated which is a good thing. But still that inconsistency of the used verb might be potentially confusing to users and being consistent here might help?

@rkoller, I also debated that a bit with myself and I thought "activate" sounds better, but I don't mind using Save and switch instead. Pushed that change to the MR :)

thanks for the fast turn around! yep i agree technically both variants work and as you said save and activate "slightly" sounds better but in regards of consistency Save and switch is the preferable choice imho. plus i've just noticed back then in the ux review we've also agreed on Save and switch (ref #15 📌 Creating a new workspace should also switch to it Needs work ). So this issue looks good to go from a manual testing perspective, but not sure if i am eligible to rtbc it or if still someone has to take another look at the code?

@rkoller, thank you as well for testing! If you feel comfortable with reviewing the code changes, then you can also RTBC the issue, otherwise you can leave that part for someone else. A core committer will ultimately review the code anyway :)

I'll just remove the tag for now, since the usability part of the issue was reviewed and approved.

Applied some small suggestions to the MR. Will mark after the tests run.

All green!

Not entirely convinced by the ::create trick but it does remove a tonne of boilerplate and it's very easily reversible if we want to override the constructor again.

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

I’ve seen it a few times in contrib modules nothing breaks but phpstorm complains about the typehint

Automatically closed - issue fixed for 2 weeks with no activity.

amateescu → closed merge request !7089