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: Timeable Node Visibility #4817

Merged
merged 15 commits into from
Feb 9, 2024

Conversation

dlubitz
Copy link
Contributor

@dlubitz dlubitz commented Jan 5, 2024

Re-Implements the already know feature for hiding and/or showing content nodes on a specific point on time.

What has changed in comparision to the old implementation:

  • Check for exceeded date time is handled async in a background job (./flow timeablenodevisibility:execute)
  • The background job creates proper events to trigger enabling or disabling of the content.
  • No implicit hiding of nodes anymore. The configured hidden state is always respected by Neos. Just the background job changes the hidden state.
  • No extra cache handling anymore. As the background job triggers the event to change the visibility, the cache flushing is also triggered the same way as all other content changes.
  • The feature is now located in the Neos.TimableNodeVisibility package (neos/timeable-node-visibility) and not part of the Neos minimum installation anymore. So you are able to decide, if you want to enable this feature or not by adding it manually to your dependencies.

Renamed the properties:

  • _hiddenBeforeDateTime => enableAfterDateTime
  • _hiddenAfterDateTime => disableAfterDateTime

Resolves: #4759
Related: #4834 , neos/rector#42

@github-actions github-actions bot added the 9.0 label Jan 5, 2024
@dlubitz dlubitz added this to the 9.0 (ES CR) milestone Jan 9, 2024
@dlubitz dlubitz linked an issue Jan 9, 2024 that may be closed by this pull request
@dlubitz dlubitz self-assigned this Jan 9, 2024
@dlubitz dlubitz force-pushed the 90/timeable-node-visibility branch 2 times, most recently from a659875 to 087f440 Compare January 15, 2024 16:12
@dlubitz dlubitz force-pushed the 90/timeable-node-visibility branch from 087f440 to 98d151e Compare January 15, 2024 16:20
@dlubitz dlubitz changed the title FEATURE Timeable Node Visibility FEATURE: Timeable Node Visibility Jan 15, 2024
@dlubitz dlubitz marked this pull request as ready for review January 15, 2024 21:26
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.

THANK you so much! I just had a quick look over the files (except the behavioural tests ^^)

@kitsunet
Copy link
Member

Looks pretty cool, thanks for taking care! Left some comments, but I think this is nicely underway, I agree with concerns regarding performance but don't have amazing ideas right now, apart from, should/could this be a projection.

@dlubitz dlubitz requested review from kitsunet and mhsdesign January 26, 2024 16:26
@mhsdesign
Copy link
Member

mhsdesign commented Feb 4, 2024

Thanks for this feature. I just played around with a little and i can say that it works :D.

At first i had to update your branch a little and to ease installation i pushed a new temporary branch to neos so everyone can add this in their root composer json to experiment: (i actually have no clue how you got it running *g)

"neos/neos-development-collection": "dev-temp-head-90-timeable as 9.0"

A few things to be noted / discussed:

  • we should add it to the slicer https://github.com/neos/slicer so this package its splatted. ✅
  • And this package needs a readme saying this is a sub-split and i would propose also some additional docs on how to use it as people will need to install it separately. ✅
  • also i thought we discussed that we wanted to make everything opt in, so that we wont provide an override nodetypes yaml
    • on the other hand this works as expected and requires 0 configuration so top ✅
  • i noticed a problem that the useworkspace would not be marked outdated and rebased, and thus changes are actually not visible inside the ui (without a random rebase from somewhere)
  • i understand that the live workspace is $special, but maybe we should allow this feature for all root workspaces, if not already? (Or maybe just for every workspace or does this lead to some obscure state? probably?) ✅ (not part of this pr)
  • disabled vs hidden ui label: FEATURE: Timeable Node Visibility #4817 (comment) ✅ (can be changed later)

@dlubitz
Copy link
Contributor Author

dlubitz commented Feb 5, 2024

I prepared the split:
PR: neos/slicer#21
Repo: https://github.com/neos/timeable-node-visibility

@dlubitz dlubitz merged commit 5ccf70a into neos:9.0 Feb 9, 2024
9 checks passed
@dlubitz dlubitz deleted the 90/timeable-node-visibility branch February 9, 2024 16:28
@dlubitz
Copy link
Contributor Author

dlubitz commented Feb 9, 2024

Follow up:

  • Register package on packagist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Re-implement support for timed node visibility
3 participants