Skip to content

Commit

Permalink
FIX Make change tracking work again for inline editable blocks
Browse files Browse the repository at this point in the history
  • Loading branch information
GuySartorelli committed Feb 23, 2025
1 parent 4c38f21 commit c95f9a9
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 63 deletions.
4 changes: 2 additions & 2 deletions client/dist/js/bundle.js

Large diffs are not rendered by default.

68 changes: 14 additions & 54 deletions client/src/components/ElementEditor/Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { inject } from 'lib/Injector';
import i18n from 'i18n';
import classNames from 'classnames';
import { connect } from 'react-redux';
import { submit } from 'redux-form';
import { submit, isDirty } from 'redux-form';
import { loadElementFormStateName } from 'state/editor/loadElementFormStateName';
import { loadElementSchemaValue } from 'state/editor/loadElementSchemaValue';
import { publishBlockMutation } from 'state/editor/publishBlockMutation';
Expand All @@ -20,7 +20,7 @@ import { DragSource, DropTarget } from 'react-dnd';
import { getEmptyImage } from 'react-dnd-html5-backend';
import { elementDragSource, isOverTop } from 'lib/dragHelpers';
import * as toastsActions from 'state/toasts/ToastsActions';
import { addFormChanged, removeFormChanged } from 'state/unsavedForms/UnsavedFormsActions';
import getFormState from 'lib/getFormState';

export const ElementContext = createContext(null);

Expand All @@ -39,32 +39,19 @@ const Element = (props) => {
const [doPublishElementAfterSave, setDoPublishElementAfterSave] = useState(false);
const [ensureFormRendered, setEnsureFormRendered] = useState(false);
const [formHasRendered, setFormHasRendered] = useState(false);
const [doDispatchAddFormChanged, setDoDispatchAddFormChanged] = useState(false);
const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false);
const [publishBlock] = useMutation(publishBlockMutation);

const formRenderedIfNeeded = formHasRendered || !props.type.inlineEditable;

useEffect(() => {
// Note that formDirty from redux can be set to undefined after failed validation
// which is confusing as the block still has unsaved changes, hence why we create
// this state variable to track this instead
// props.formDirty is either undefined (when pristine) or an object (when dirty)
const formDirty = typeof props.formDirty !== 'undefined';
if (formDirty && !hasUnsavedChanges) {
setHasUnsavedChanges(true);
}
props.onChangeHasUnsavedChanges(props.formDirty);
}, [props.formDirty]);

useEffect(() => {
props.onChangeHasUnsavedChanges(hasUnsavedChanges);
}, [hasUnsavedChanges]);

useEffect(() => {
if (props.saveElement && hasUnsavedChanges && !doSaveElement) {
if (props.saveElement && props.formDirty && !doSaveElement) {
setDoSaveElement(true);
}
}, [props.saveElement, hasUnsavedChanges, props.increment]);
}, [props.saveElement, props.formDirty, props.increment]);

useEffect(() => {
if (props.connectDragPreview) {
Expand All @@ -81,7 +68,7 @@ const Element = (props) => {
useEffect(() => {
if (justClickedPublishButton && formRenderedIfNeeded) {
setJustClickedPublishButton(false);
if (hasUnsavedChanges) {
if (props.formDirty) {
// Save the element first before publishing, which may trigger validation errors
props.submitForm();
setDoPublishElementAfterSave(true);
Expand All @@ -92,13 +79,6 @@ const Element = (props) => {
}
}, [justClickedPublishButton, formHasRendered]);

useEffect(() => {
if (doDispatchAddFormChanged) {
setDoDispatchAddFormChanged(false);
props.dispatchAddFormChanged();
}
}, [doDispatchAddFormChanged]);

const getNoTitle = () => i18n.inject(
i18n._t('ElementHeader.NOTITLE', 'Untitled {type} block'),
{ type: props.type.title }
Expand Down Expand Up @@ -143,18 +123,7 @@ const Element = (props) => {
showPublishedElementToast(wasError);
setDoPublishElement(false);
setDoPublishElementAfterSave(false);
// Ensure that formDirty becomes falsey after publishing
// We need to call at a later render rather than straight away or redux-form may override this
// and set the form state to dirty under certain conditions
// setTimeout is a hackish way to do this, though I'm not sure how else we can do this
// The core issue is that redux-form will detect changes when a form is hydrated for the first
// time under certain conditions, specifically during a behat test when trying to publish a closed
// block when presumably the apollo cache is empty (or something like that). This happens late and
// there are no hooks/callbacks available after this happens the input onchange handlers are fired
Promise.all(refetchElementalArea())
.then(() => {
setTimeout(() => props.dispatchRemoveFormChanged(), 250);
});
refetchElementalArea();
};

// Save action
Expand Down Expand Up @@ -337,10 +306,6 @@ const Element = (props) => {
if (props.type.inlineEditable) {
setPreviewExpanded(true);
}
// Ensure that formDirty remains truthy
// Note we need to call props.dispatchAddFormChanged() on the next render rather than straight away
// or it will get unset by code somewhere else, probably redux-form
setDoDispatchAddFormChanged(true);
// Don't accidentally auto publish the element once validation errors are fixed
if (doPublishElementAfterSave) {
setDoPublishElementAfterSave(false);
Expand All @@ -349,7 +314,6 @@ const Element = (props) => {
return;
}
// Form is valid
setHasUnsavedChanges(false);
setNewTitle(title);
if (doPublishElementAfterSave) {
setDoPublishElementAfterSave(false);
Expand Down Expand Up @@ -435,6 +399,7 @@ const Element = (props) => {
broken={type.broken}
onFormSchemaSubmitResponse={handleFormSchemaSubmitResponse}
onFormInit={() => handleFormInit(activeTab)}
formDirty={formDirty}
/>
</ElementContext.Provider>
</div>);
Expand Down Expand Up @@ -462,7 +427,12 @@ function mapStateToProps(state, ownProps) {

const tabSetName = tabSet && tabSet.id;
const uniqueFieldId = `element.${elementName}__${tabSetName}`;
const formDirty = state.unsavedForms.find((unsaved) => unsaved.name === `element.${elementName}`);

const formName = loadElementFormStateName(ownProps.element.id);
let formDirty = isDirty(`element.${formName}`, getFormState)(state);
if (typeof formDirty === 'undefined') {
formDirty = true;
}

// Find name of the active tab in the tab set
// Only defined once an element form is expanded for the first time
Expand Down Expand Up @@ -490,16 +460,6 @@ function mapDispatchToProps(dispatch, ownProps) {
// Perform a redux-form remote-submit
dispatch(submit(`element.${elementName}`));
},
dispatchAddFormChanged() {
// Ensures the form identifier is in unsavedForms in the global redux state
// This is used to derive the formDirty prop in mapStateToProps
dispatch(addFormChanged(`element.${elementName}`));
},
dispatchRemoveFormChanged() {
// Removes the form identifier from unsavedForms in the global redux store
// Opposite of beheaviour of dispatchAddFormChanged()
dispatch(removeFormChanged(`element.${elementName}`));
},
actions: {
toasts: bindActionCreators(toastsActions, dispatch),
},
Expand Down
8 changes: 8 additions & 0 deletions client/src/components/ElementEditor/ElementEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ class ElementEditor extends PureComponent {

render() {
const {
fieldName,
formState,
ToolbarComponent,
ListComponent,
areaId,
Expand Down Expand Up @@ -104,6 +106,12 @@ class ElementEditor extends PureComponent {
sharedObject={sharedObject}
/>
<ElementDragPreview elementTypes={elementTypes} />
<input
name={fieldName}
type="hidden"
value={JSON.stringify(formState) || ''}
className="no-change-track"
/>
</div>
);
}
Expand Down
43 changes: 43 additions & 0 deletions tests/Behat/features/file-upload.feature
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,54 @@ Feature: Files can be saved in and removed from elemental blocks
And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\FileElementalExtension" to the "DNADesign\Elemental\Models\ElementContent" class
And I go to "/dev/build?flush"
And a "image" "file1.jpg"
And a "image" "file2.jpg"
And a "page" "Blocks Page" with a "My title" content element with "My content" content
And the "group" "EDITOR" has permissions "Access to 'Pages' section"
And I am logged in as a member of "EDITOR" group
And I go to "/admin/pages"
And I follow "Blocks Page"

Scenario: Add a file and save the block, then remove it and add a different one
# Add a file to the block
Given I click on the caret button for block 1
Then I should not see "file1"
When I click "Choose existing" in the "#Form_ElementForm_1 .uploadfield" element
And I press the "Back" HTML field button
And I click on the file named "file1" in the gallery
And I press the "Insert" button
And I press the "View actions" button
And I click on the ".element-editor__actions-save" element
Then I should see a "Saved 'My title' successfully" success toast
# Check we see the file both in the current page load (react state is correct) and after reloading the form
Then I should see "file1"
When I go to "/admin/pages"
And I follow "Blocks Page"
And I click on the caret button for block 1
Then I should see "file1"
# Then remove the file from the block
And I click on the "#Form_ElementForm_1 .uploadfield-item__remove-btn" element
Then I should not see "file1"
# Try adding the same file back
When I click "Choose existing" in the "#Form_ElementForm_1 .uploadfield" element
And I press the "Back" HTML field button
And I click on the file named "file1" in the gallery
And I press the "Insert" button
And I press the "View actions" button
# same file, so we shouldn't see the button
Then I should not see the save button for block 1
# Add a different file
And I click on the "#Form_ElementForm_1 .uploadfield-item__remove-btn" element
When I click "Choose existing" in the "#Form_ElementForm_1 .uploadfield" element
# Note we don't have to press "Back" here because react knows what folder we were just in before
And I click on the file named "file2" in the gallery
And I press the "Insert" button
And I press the "View actions" button
Then I should see the save button for block 1
And I click on the ".element-editor__actions-save" element
Then I should see a "Saved 'My title' successfully" success toast
And I should see "file2"
And I should not see "file1"

Scenario: Add a file and save the block, then remove the file and save the block
# Add a file to the block
Given I click on the caret button for block 1
Expand All @@ -33,6 +75,7 @@ Feature: Files can be saved in and removed from elemental blocks
Then I should see "file1"
# Then remove the file from the block
And I click on the "#Form_ElementForm_1 .uploadfield-item__remove-btn" element
Then I should not see "file1"
And I press the "View actions" button
And I click on the ".element-editor__actions-save" element
Then I should see a "Saved 'My title' successfully" success toast
Expand Down
18 changes: 11 additions & 7 deletions tests/Behat/features/inline-block-validation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Feature: Blocks are validated when inline saving individual blocks
And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\ElementContentExtension" to the "DNADesign\Elemental\Models\ElementContent" class
And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\NumericFieldExtension" to the "SilverStripe\Forms\NumericField" class
And a "image" "file1.jpg"
And a "image" "file2.jpg"
And I go to "/dev/build?flush"
And a "page" "Blocks Page" with a "My title" content element with "My content" content
And the "group" "EDITOR" has permissions "Access to 'Pages' section"
Expand All @@ -34,6 +35,11 @@ Feature: Blocks are validated when inline saving individual blocks
When I press the "View actions" button
And I click on the ".element-editor__actions-save" element
Then I should see "Title cannot be x" in the ".form__validation-message" element
# Reset value to something valid and resave - validates saving after validation message works
# Specifically swapping back to the original value
And I fill in "My title" for "Title" for block 1
And I wait for 1 second
And I press the "Save" button

Scenario: General validation error
When I fill in "z" for "Title" for block 1
Expand All @@ -47,11 +53,10 @@ Feature: Blocks are validated when inline saving individual blocks
When I press the "View actions" button
And I click on the ".element-editor__actions-save" element
Then I should see "This field cannot be 1" in the ".form__validation-message" element
# Reset value to something valid to prevent "unsaved changes" alert
# Reset value to something valid and resave - validates saving after validation message works
# Specifically swapping to a new value
Then I fill in "2" for "My Int" for block 1
# Ensure react field is filled in before submitting
And I wait for 1 second
# Need to save the whole page to stop the alert
And I press the "Save" button

Scenario: Related data validation error with ID suffix (MyPageID)
Expand All @@ -72,7 +77,7 @@ Feature: Blocks are validated when inline saving individual blocks
And I click on the ".element-editor__actions-save" element
Then I should see "\"My File\" is required" in the ".form__validation-message" element
When I click "Choose existing" in the ".uploadfield" element
And I click on the file named "file1" in the gallery
And I click on the file named "file2" in the gallery
And I press the "Insert" button
And I press the "View actions" button
And I click on the ".element-editor__actions-save" element
Expand All @@ -92,9 +97,8 @@ Feature: Blocks are validated when inline saving individual blocks
And I click on the ".element-editor__actions-save" element
Then I should see "My Field"
And I should see "MyField cannot be x" in the ".form__validation-message" element
# Reset value to something valid to prevent "unsaved changes" alert
# Reset value to something valid and resave - validates saving after validation message works
# and avoids an alert after the test ends
Then I fill in "abc" for "My Field" for block 1
# Ensure react field is filled in before submitting
And I wait for 1 second
# Need to save the whole page to stop the alert
And I press the "Save" button
Binary file added tests/Behat/files/file2.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit c95f9a9

Please sign in to comment.