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: Lock the UI and present a modal during publish/discard #3759

Merged
merged 34 commits into from
Apr 25, 2024

Conversation

grebaldi
Copy link
Contributor

@grebaldi grebaldi commented Mar 19, 2024

solves: #3743
builds on: #3752

The PublishDiscardDialog is a modal that is supposed to pop up whenever an editor starts publishing or discarding their workspace.

Editors need to confirm their intent, after which they are presented with a process indicator that blocks the screen as long as the remote operation is not finished yet.

Finally, Editors see a success message or an error message - depending on the outcome. In both cases, they need to acknowledge the info. If they see an error message, they are also given an opportunity to retry the action.

Publish Workflow

Show Video
Peek.2024-03-22.16-25.Publish.workflow.mp4

For publishing, users are now first presented with a confirmation dialog. Formerly, publishing didn't show this dialog. This PR makes both Publish and Discard consistent in this regard (as you wouldn't want to accidentally publish anything either).

If the user confirms the action, they see an uncancellable modal that illustrates the ongoing operation. Formerly, the operation was only indicated by a spinning icon alongside the PublishingDropdown head. The UI remained usable during the operation, which could potentially lead to invalid actions. Now, the UI is locked, including a beforeunload handler that warns the user if they're about to reload or leave the page.

The progress indicator remains open for as long as any async operation is still running. This includes the publishing operation itself, but also the adjacent reload of nodes and the reload of the guest frame. Only after the UI has reached a stable and consistent state, the lock is released.

Finally, the user is presented with a dialog that lets them acknowledge the operation. Formerly, a successful publishing process would have been indicated by a fleeting flash message. The acknowledgement dialog remains open until the user has taken notice. This way users can safely leave the tab, do something else, and return without losing the context of what their last action has been.

Discard Workflow

Show Video
Peek.2024-03-22.16-24.Discard.Workflow.mp4

The Discard workflow behaves exactly like the Publish workflow. The only difference is in the coloring and the progress indicator illustration.

Error Handling

Show Video
Peek.2024-03-22.16-27.Error.Workflow.mp4

Formerly, errors during Publish or Discard would have been displayed using a persistent flash message. This PR introduces a new component that can be embedded inside a dialog and that shows more detailed error information in FLOW_CONTEXT=Development.

Also new: Users are offered a "Try Again" button, when Publish or Discard fails.

Remaining TODOs

  • Create <PublishDiscardDialog/>
  • Move old I18n labels into Neos.Neos.Ui and initialize new ones
  • Create Follow-Up issue or PR for labels in Neos.Neos
  • Wire up the PublishDiscardDialog component with redux
  • Split out CR.Publishing state from CR.Workspaces
    • Summarize actions abortPublish, confirmPublish, finishPublish, acknowledgePublish, abortDiscard, confirmDiscard, finishDiscard, acknowledgeDiscard -> cancel, confirm, finish, acknowledge
  • Rename PublishDiscard -> Publishing everywhere
    • Rename PublishDiscardDialog -> PublishingDialog
    • Rename PublishDiscardMode -> PublishingMode
      • Rename PublishDiscardMode.PUBLISHING -> PublishingMode.PUBLISH
      • Rename PublishDiscardMode.DISCARDING -> PublishingMode.DISCARD
    • Rename PublishDiscardScope -> PublishingScope
    • Rename PublishDiscardPhase -> PublishingPhase
  • Merge publish and discard saga
  • Remove ad-hoc NODE_HAS_BEEN_CREATED constant and make TypeOfChange an enum
  • Better Error Handling
    • Use direct responses from publish/discard endpoints rather than FeedbackCollection to gain better control of error handling
    • Handle JavaScript errors more extensively in publishing saga
    • Switch action buttons in error case and rename: [Try again] [Okay] -> [Cancel] [Try again]
  • Implement Retry
  • Refer to target workspace name in success message
  • Adjust E2E tests
  • Remove legacy <DiscardDialog/>
  • Remove legacy actions.UI.Remote actions
    • Remove startPublishing()
    • Remove finishPublishing()
    • Remove startDiscarding()
    • Remove finishDiscarding()
  • Add beforeunload handler during publish/discard
  • Fetch updated workspace info from server after publish/discard
  • !!! Revert commit 79de33b !!!
  • Fix: All 0 change(s) in workspace "user-admin" were sucessfully discarded.

Follow-Ups

@grebaldi grebaldi linked an issue Mar 19, 2024 that may be closed by this pull request
4 tasks
@mhsdesign mhsdesign self-requested a review March 21, 2024 11:17
@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch from 6f73c45 to c25910d Compare March 22, 2024 10:39
@mhsdesign
Copy link
Member

Wooooooooow 🥳 the dialog looks super cool ;)

@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch from ec95f47 to 7f50bb0 Compare March 28, 2024 14:49
@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch from 7f50bb0 to 25e88e0 Compare March 29, 2024 12:29
Base automatically changed from publishingBonanza to 9.0 April 5, 2024 18:24
@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch 3 times, most recently from 5a8a92c to 6645c4d Compare April 10, 2024 14:31
grebaldi added a commit to neos/neos-development-collection that referenced this pull request Apr 12, 2024
This removes the following labels:

Neos.Neos:Main:content.components.discardAllDialog.discardXChangesSubheader
Neos.Neos:Main:content.components.discardAllDialog.discardAllChangesHeader
Neos.Neos:Main:content.components.discardAllDialog.discardAllChangesSubheader
Neos.Neos:Main:content.components.publishAllDialog.header
Neos.Neos:Main:content.components.publishAllDialog.subheader

The publishing workflow in the UI has changes significantly and requires
an entirely new set of labels. Those labels are now located in the UI
repository.
@grebaldi
Copy link
Contributor Author

Unit test failures are unrelated. They are due to a broken jest snapshot that has been fixed with #3757 for 8.3. The fix hasn't arrived in 9.0 yet, so unit tests will be green after upmerges.

@grebaldi grebaldi marked this pull request as ready for review April 12, 2024 11:31
@grebaldi grebaldi changed the title WIP: Lock the UI and present a modal during publish/discard !!!FEATURE: Lock the UI and present a modal during publish/discard Apr 12, 2024
Copy link
Member

@nezaniel nezaniel left a comment

Choose a reason for hiding this comment

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

Love it!

I added a few nitpicks and am otherwise ready to approve

Classes/Application/ReloadNodes/MinimalNodeForTree.php Outdated Show resolved Hide resolved
Classes/Controller/BackendServiceController.php Outdated Show resolved Hide resolved
Classes/Controller/BackendServiceController.php Outdated Show resolved Hide resolved
grebaldi added a commit that referenced this pull request Apr 17, 2024
grebaldi added a commit that referenced this pull request Apr 17, 2024
...to retrieve the base workspace during publish opertaions.

As suggested by @nezaniel (see:
#3759 (comment))
@grebaldi grebaldi requested a review from nezaniel April 22, 2024 09:35
This package provides utilities to analyze errors of various types. It
provides the `<ErrorView/>` component that can be embedded in other
components to display those errors. In `FLOW_CONTEXT=Development` the
`<ErrorView/>` component will also display meta information (e.g.
exception class name, stack trace, etc.).
The PublishDiscardDialog is a modal that is supposed to pop up whenever
an editor starts publishing or discarding their workspace.

Editors need to confirm their intent, after which they are presented
with a process indicator that blocks the screen as long as the remote
operation is not finished yet.

Finally, Editors see a success message or an error message - depending
on the outcome. In both cases, they need to acknowledge the info. If
they see an error message, they are also given an opportunity to retry
the action.

This commit creates the visual implementation of the dialog but does not
wire it to the application state yet.
@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch from bec21c1 to 455ff67 Compare April 22, 2024 15:21
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.

Wow nice work so much work thanks. We are lucky to have you on the team as this is some crazy state handling ;)

After reading through im not sure i understand even half of it but it looks legit ^^
I will experiment locally and - if you find the time - would like to chat with you what changed but in human language ;)

$nodeMapBuilder->addNode($siteNode);

$gatherNodesRecursively = function (
&$nodeMapBuilder,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
&$nodeMapBuilder,
$nodeMapBuilder,

$ancestors,
$subgraph
) {
if ($level < $this->loadingDepth || // load all nodes within loadingDepth
Copy link
Member

Choose a reason for hiding this comment

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

ah i see this logic is not new but moved from NeosUiDefaultNodesOperation ...

Copy link
Member

Choose a reason for hiding this comment

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

... but we dont seem to remove the NeosUiDefaultNodesOperation yet?

@@ -0,0 +1,41 @@
# @neos-project/neos-ui-error
Copy link
Member

Choose a reason for hiding this comment

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

nice idea to add some structure to our codebase :D

Should this package also contain the default error boundary? neos-ui/src/Containers/ErrorBoundary ... but on the other hand i guess there is little code to share and the boundary can stay were it wants

@@ -73,12 +47,52 @@ export interface State {
//
// Export the actionTypes
//
export const actionTypes = typedKeys(all).reduce((acc, cur) => ({...acc, [cur]: all[cur].actionTypes}), {});
export const actionTypes = {
Copy link
Member

Choose a reason for hiding this comment

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

nice cleanup that is some next level stuff otherwise 😅

packages/neos-ui-sagas/src/Publish/index.ts Show resolved Hide resolved
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.

Thanks for walking through this one with me and explaining everything ^^

}
};

const reloadAfterPublishing = makeReloadAfterDiscard({routes});
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: the factory should also be renamed ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course :D Thanks for the pointer!

@mhsdesign
Copy link
Member

Okay perfect ❤️ again many thanks. Please feel free to hit the merge button. (The failures are unrelated and will be fixed by a rebase)

@grebaldi grebaldi merged commit 2485e9e into 9.0 Apr 25, 2024
8 of 10 checks passed
@grebaldi grebaldi deleted the feature/conflict-resolution-01/publishing-modal branch April 25, 2024 15:11
kdambekalns added a commit to neos/neos-development-collection that referenced this pull request Jun 5, 2024
…te-i18n-labels

TASK: Remove i18n labels that are obsolete after neos/neos-ui#3759
neos-bot pushed a commit to neos/neos that referenced this pull request Jun 5, 2024
This removes the following labels:

Neos.Neos:Main:content.components.discardAllDialog.discardXChangesSubheader
Neos.Neos:Main:content.components.discardAllDialog.discardAllChangesHeader
Neos.Neos:Main:content.components.discardAllDialog.discardAllChangesSubheader
Neos.Neos:Main:content.components.publishAllDialog.header
Neos.Neos:Main:content.components.publishAllDialog.subheader

The publishing workflow in the UI has changes significantly and requires
an entirely new set of labels. Those labels are now located in the UI
repository.
neos-bot pushed a commit to neos/neos that referenced this pull request Jun 5, 2024
…te-i18n-labels

TASK: Remove i18n labels that are obsolete after neos/neos-ui#3759
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FEATURE: Lock the UI and present a modal during publish/discard
3 participants