Skip to content

BUGFIX: Use correct color for timed visibility icon #4003

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: 9.0
Choose a base branch
from
Open

BUGFIX: Use correct color for timed visibility icon #4003

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions packages/neos-ui/src/Containers/LeftSideBar/NodeTree/Node/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import React, {PureComponent} from 'react';
import PropTypes from 'prop-types';
import {connect} from 'react-redux';

import moment from 'moment';
import hashSum from 'hash-sum';
import flowright from 'lodash.flowright';

import {Tree, Icon} from '@neos-project/react-ui-components';
import {stripTags, decodeHtml} from '@neos-project/utils-helpers';

import {actions, selectors} from '@neos-project/neos-ui-redux-store';
import {isNodeCollapsed} from '@neos-project/neos-ui-redux-store/src/CR/Nodes/helpers';
import {neos} from '@neos-project/neos-ui-decorators';

import hashSum from 'hash-sum';
import {urlWithParams} from '@neos-project/utils-helpers/src/urlWithParams';

const getContextPath = node => node?.contextPath;
Expand Down Expand Up @@ -172,7 +171,13 @@
const hasTimeableNodeVisibility = node?.properties?._hasTimeableNodeVisibility;

if (hasTimeableNodeVisibility) {
const circleColor = isDisabled ? 'error' : 'primaryBlue';
Copy link
Contributor

Choose a reason for hiding this comment

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

The internal logic of timable node visibility has changed. That's why this also has changed. The state is rather depending on the actual visibility state (disabled/enabled) than the "calculated" state based on the datetime.

https://github.com/neos/timeable-node-visibility

Copy link
Member Author

Choose a reason for hiding this comment

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

But that also means, the editor has to wait for the job to run so they can see the real state.

What happens if they set hidden manually, the job would then change the value, correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, correct. As I wrote in the issue:

IIRC we discussed a feature for the UI to show a warning if the current state is not correct (e.g node is visible, but the enableAfter is in future etc). But I couldn't find any issue for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could use this change for that, as it would be more or less the same code ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess so. Would be a feature and we would need some idea how we want to display that.

Copy link
Contributor

Choose a reason for hiding this comment

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

But I think your solution does not cover all scenatios atm. Please check the tests in the neos/timeable-node-visibility package: https://github.com/neos/timeable-node-visibility/blob/9.0/Tests/Behavior/Features/HandleExceeded.feature

Copy link
Contributor

Choose a reason for hiding this comment

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

And the backend implementation to determine the state: https://github.com/neos/timeable-node-visibility/blob/9.0/Classes/Service/TimeableNodeVisibilityService.php#L129-L154

const {enableAfterDateTime, disableAfterDateTime} = node.properties;

let isCurrentlyHidden = isDisabled

Check failure on line 176 in packages/neos-ui/src/Containers/LeftSideBar/NodeTree/Node/index.js

View workflow job for this annotation

GitHub Actions / Code style 

'isCurrentlyHidden' is never reassigned. Use 'const' instead
|| (enableAfterDateTime && moment(enableAfterDateTime).isAfter(moment()))
|| (disableAfterDateTime && moment(disableAfterDateTime).isBefore(moment()));

const circleColor = isDisabled || isCurrentlyHidden ? 'error' : 'primaryBlue';

return (
<span className="fa-layers fa-fw">
Expand Down
Loading