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: Reimplement EditPreviewModes for Neos 9 #4505

Merged
merged 6 commits into from
Sep 16, 2023

Conversation

mficzel
Copy link
Member

@mficzel mficzel commented Sep 12, 2023

Since nodes have no context in Neos 9 the edit preview mode cannot be decided by asking the node any more.

This change renovates the UserInterfaceMode and the UserInterfaceModeService to use php 8 value objects now named RenderingMode and RenderingModeService. The renderingModeName is added as argument to the NodeController::previewAction that will pass this as an option to the Neos\Neos\FusionView if no renderingModeName the RenderingModeService will determine the currently selected mode of the user.

The renderingMode is then added to the fusionGlobals by the FusionView and is used for checking in various places in fusion.

Resolves: #4086
Resolves: #4396
Based on new Fusion globals: #4425

Upgrade instructions

The uses of the following fusion expressions have to be adjusted as follows:

  • node.context.live - !renderingMode.isEdit *
  • node.context.inBackend - renderingMode.isEdit *
  • node.context.currentRenderingMode.edit - renderingMode.isEdit
  • node.context.currentRenderingMode.preview - renderingMode.isPreview
  • node.context.currentRenderingMode.name - renderingMode.name

* this condition should cover the usecase most of the time. For a more accurate representation of "inBackend" use ${renderingMode.isEdit || renderingMode.isPreview}

Review instructions

See #4396 for a full explanation about the decision to make the backend check node independent.

This is an newer approach to pr #4067

Follow up Todos:

  • The change TASK: Adjust migration for renderingMode.isEdit rector#22 adjusts the rector migrations to use the new renderingMode global. This one should be merged after this pr.
  • Remove Neos.Node.inBackend() helper after the UI stopped using it
  • Consider to adjusting the setting keys editPreviewModes and defaultEditPreviewMode to renderingModes and defaultRenderingMode in a separate pr

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@mficzel
Copy link
Member Author

mficzel commented Sep 12, 2023

Since this falls back to UserInterfaceService->findUserInterfaceModeNameByCurrentUser it actually allows to switch the mode in the ui. 🎉

Still think that in the long run the uiMode should be passed to the previewAction (parameter is there) and that even editAction and previewAction should be separates. However both are not necessary to let Neos 9 run and can be added later.

@mficzel mficzel force-pushed the 90/editPreviewModeSupport_v3 branch 2 times, most recently from 965cf6e to 5439b86 Compare September 12, 2023 20:37
@mficzel
Copy link
Member Author

mficzel commented Sep 12, 2023

A matter to discuss is how to name the properties of the edit preview mode and how they should be accessed. The pr is marked as draft to allow us the short but needed discussion

possible options include:

  • userInterfaceMode.edit|preview|live|name - currently implemented and my favorite
  • Neos.uiMode.edit|preview|live|name
  • Neos.uiMode.edit()|preview()|live()|name()
  • userInterfaceMode.isEditMode|isPreviewMode|isLive|name`

We should discuss and decide this ideally once as this requires adjustments in a dozen fusion files.

@mficzel mficzel force-pushed the 90/editPreviewModeSupport_v3 branch 5 times, most recently from ca9a574 to 2a05eeb Compare September 13, 2023 05:44
@mficzel mficzel requested a review from grebaldi September 13, 2023 05:53
@mhsdesign
Copy link
Member

Hi @mficzel thanks for your 3rd implementation ❤️ !!!

About the naming, we did discuss a few options in the last weekly's (when your were on vacation) and i think most arguments are written down in this thread: https://neos-project.slack.com/archives/C3MCBK6S2/p1693988493145879 and in this issue: #4086 (comment)

I want to suggest that we both have a little call to discuss all arguments on the table and then we pick the best solution?

@mficzel mficzel force-pushed the 90/editPreviewModeSupport_v3 branch from 2a05eeb to 690f52e Compare September 14, 2023 17:22
mficzel added a commit to neos/rector that referenced this pull request Sep 14, 2023
the migration previously adjusted the code to a now deprecated eel helper. In pr neos/neos-development-collection#4505 the userInterfaceMode is introduced as global fusionValue which made an adjustment in the rector necessary.

`(node|documentNode|site).context.inBackend` is now migrated to `userInterfaceMode.isEdit`
`(node|documentNode|site).context.live` is now migrated to `userInterfaceMode.isLive`

In cases where it cannot be determined wether a node is affected by the `context` operation a comment is added as before.
@mficzel mficzel marked this pull request as ready for review September 14, 2023 17:42
mficzel added a commit to neos/rector that referenced this pull request Sep 14, 2023
the migration previously adjusted the code to a now deprecated eel helper. In pr neos/neos-development-collection#4505 the userInterfaceMode is introduced as global fusionValue which made an adjustment in the rector necessary.

`(node|documentNode|site).context.inBackend` is now migrated to `userInterfaceMode.isEdit`
`(node|documentNode|site).context.live` is now migrated to `userInterfaceMode.isLive`

In cases where it cannot be determined wether a node is affected by the `context` operation a comment is added as before.
@mficzel mficzel force-pushed the 90/editPreviewModeSupport_v3 branch 2 times, most recently from a2ce30e to 1729616 Compare September 14, 2023 18:43
@mficzel
Copy link
Member Author

mficzel commented Sep 14, 2023

Rename:

  • userInterfaceMode -> renderingMode (as in fe there is no ui)
  • live -> frontend (to avoid confusion with live workspace)

@mficzel mficzel force-pushed the 90/editPreviewModeSupport_v3 branch from ce17ea1 to 6d00b3e Compare September 15, 2023 11:51
@mficzel mficzel changed the title FEATURE: Reimplement EditPreviewModes for Neos 9 !!! FEATURE: Reimplement EditPreviewModes for Neos 9 Sep 15, 2023
@@ -120,8 +125,14 @@ class NodeController extends ActionController
* with unsafe requests from widgets or plugins that are rendered on the node
* - For those the CSRF token is validated on the sub-request, so it is safe to be skipped here
*/
public function previewAction(string $node): void
public function previewAction(string $node, string $renderingModeName = null): void
Copy link
Member

Choose a reason for hiding this comment

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

FYI, the ui will currently not use this parameter yet (it could via neos/neos-ui#3411)

But importantly this change allows us to open a page in a new tab independent of the session's value.
Although this would only work for this one page, successive links will currently not have that query param added - the linking service still needs to lear this, but this is out of scope of this pr.

Otherwise, this feature would only work for this one page, successive links will currently not have that query param added - the linking service still needs to learn this, but we delay this for another time.
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.

I found some nitpicks, and proposed a solution for those here: #4521

@mhsdesign
Copy link
Member

The preview via this link /neos/redirect?node=user-admin__eyJsYW5ndWFnZSI6ImVuX1VTIn0%3D__452374b3-3580-2af3-71bd-f9932faea84d doesnt work yet but because its not implemented in the ui:

image

https://github.com/neos/neos-ui/blob/17361011b1adc5367d77dea632f97df9f7f7703c/Classes/Controller/BackendController.php#L209-L224

@mhsdesign
Copy link
Member

mhsdesign commented Sep 15, 2023

Works so far great inside the ui. I can also confirm that changing the mode via user preferences/ session works and all tabs get the new mode (basically this behavior, which we might want to fix at some point #965).

But the linking seems a bit off in the ui, and when you open the iframe in a new tab (use the preview endpoint /neos/preview?node=user-admin__123)

image

Isnt this that issue? #2347 so no new problem ...

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.

We discussed some optional follow ups, but this pr is great as it is and can be merged!

@mhsdesign
Copy link
Member

As the solution was discussed more than 5 times in our weekly’s and additionally @mficzel and me spend time discussing it, i would say this is approach is reviewed/approved enough to be merged with one formal GitHub approval.

@mficzel mficzel merged commit 96e4257 into 9.0 Sep 16, 2023
5 checks passed
@mficzel mficzel deleted the 90/editPreviewModeSupport_v3 branch September 16, 2023 16:41
mficzel added a commit that referenced this pull request Sep 20, 2023
It was deprecated in #4505 in favor of `${userInterfaceMode.isEdit}`.
Since the ui is adjusted in neos/neos-ui#3618 it can now be removed.
mficzel added a commit that referenced this pull request Sep 20, 2023
It was deprecated in #4505 in favor of `${userInterfaceMode.isEdit}`.
Since the ui is adjusted in neos/neos-ui#3618 it can now be removed.
ahaeslich added a commit that referenced this pull request Sep 21, 2023
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Jan 12, 2024
neos-bot pushed a commit to neos/neos that referenced this pull request Jan 31, 2024
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Feb 15, 2024
The TypoScript context variable `editPreviewMode` was deprecated in favour of `${documentNode.context.currentRenderingMode.name}`

 With neos 9 that again was removed in favour of the fusion global `renderingMode`.

neos#4505
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Feb 16, 2024
The TypoScript context variable `editPreviewMode` was deprecated in favour of `${documentNode.context.currentRenderingMode.name}`

 With neos 9 that again was removed in favour of the fusion global `renderingMode`.

neos#4505
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Feb 16, 2024
The TypoScript context variable `editPreviewMode` was deprecated in favour of `${documentNode.context.currentRenderingMode.name}`

 With neos 9 that again was removed in favour of the fusion global `renderingMode`.

neos#4505
neos-bot pushed a commit to neos/neos that referenced this pull request Feb 16, 2024
The TypoScript context variable `editPreviewMode` was deprecated in favour of `${documentNode.context.currentRenderingMode.name}`

 With neos 9 that again was removed in favour of the fusion global `renderingMode`.

neos/neos-development-collection#4505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FEATURE: Fusion renderingMode semantic correct inBackend 9.0: Edit Preview Mode Support
2 participants