Skip to content
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

FEATURE: Add workspace Sync button #3665

Merged
merged 11 commits into from
Dec 22, 2023
Merged

Conversation

pKallert
Copy link
Contributor

@pKallert pKallert commented Nov 14, 2023

This will add a rebase button to the UI for rebasing the personal workspace of a user

What I did
I added a button to the Neos UI that shows up if the personal workspace of a user is outdated or in error state.

How I did it
Neos UI when personal workspace is outdated:
Bildschirmfoto 2023-11-24 um 12 04 32

Clicking on the button shows popup:
Bildschirmfoto 2023-11-24 um 11 57 55

Clicking on "Synchronize now" will rebase the workspace

Neos UI when personal workspace is outdated and in error mode:
Bildschirmfoto 2023-11-24 um 12 03 03

Clicking on the button shows popup:
Bildschirmfoto 2023-11-24 um 12 03 14

Related: #3803

@github-actions github-actions bot added the 9.0 label Nov 14, 2023
@crydotsnake crydotsnake added the Feature Label to mark the change as feature label Nov 22, 2023
@crydotsnake crydotsnake changed the title Draft: Add workspace Sync button Draft: FEATURE: Add workspace Sync button Nov 22, 2023
@crydotsnake crydotsnake changed the title Draft: FEATURE: Add workspace Sync button FEATURE: Add workspace Sync button Nov 22, 2023
@crydotsnake crydotsnake marked this pull request as draft November 22, 2023 07:51
@ahaeslich ahaeslich marked this pull request as ready for review November 24, 2023 08:12
@markusguenther markusguenther self-assigned this Dec 5, 2023
@markusguenther
Copy link
Member

I replaced the custom Icons with default fontawesome icons. As the inner circle is too small for additional signs, I added a badge as a layer. The result looks now like that:

Up-to-date:
Screenshot 2023-12-11 at 13 28 27

Outdated but can be rebased:
Screenshot 2023-12-11 at 13 28 06

Conflict:
Screenshot 2023-12-11 at 13 17 43

Also, the dialog looks a bit different now.

Outdated but can be rebased:
Screenshot 2023-12-11 at 15 54 05

Conflict:
Screenshot 2023-12-11 at 15 54 34

@markusguenther
Copy link
Member

I also taking care of the end to end tests.

@markusguenther
Copy link
Member

Is it OK that I additionally mark the workspace as read only, so that users cannot edit content when the workspace is falling in a conflicting state? It makes no sense to be able to edit content when it cannot be used anyway.

@mhsdesign mhsdesign self-requested a review December 21, 2023 09:33
/**
* Rebase the user workspace
*/
const rebaseWorkspace = (name: string) => createAction(actionTypes.REBASE_WORKSPACE, name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there no reducer needed for REBASE_WORKSPACE? i guess because its handled via saga?

Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good so far and i also like the icon and gui ❤️

But i noticed some things while testing:

  • if changes were made to the currently viewed page and a rebase is excecuted the document must be reloaded (and maybe also the page tree generally?)
  • when rebasing there are two spinners which seems odd?
image - one can only rebase the user workspace and detect conflicts there. But if the base workspace is a custom workspace and not live and it is outdated or in conflict the ui doesnt tell

Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really judge on the ReactJS parts though and I didn't get around testing this yet but everything that I understand looks really neat and great! :)

}

$success = new Success();
$success->setMessage(sprintf('Successfully synced User workspace'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that something that needs to be localized?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed will take care later!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be that those messages are not a flashmessage but just show up in the console as debug info in which case translation is irrelevant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably then also localize all of the other $success->setMessage calls in the Controller to be consistent

@markusguenther
Copy link
Member

@mhsdesign the spinner is a known issue but as it reacts on the same states we would need a new state then for the rebase in process 🤷‍♂️

@markusguenther
Copy link
Member

@bwaidelich @mhsdesign Hope that I could fulfil your wishes :), we have now translator labels and an extra remote state and therefore also a reducer, so now the indicator is just for the rebase.

@mhsdesign mhsdesign merged commit 654072c into neos:9.0 Dec 22, 2023
10 checks passed
Copy link
Contributor

@dlubitz dlubitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me by reading! Thank you @pKallert for taking care!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9.0 Feature Label to mark the change as feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants