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

ENH Refactor Element to use mutation hook #1174

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4,274 changes: 4,264 additions & 10 deletions client/dist/js/bundle.js

Large diffs are not rendered by default.

499 changes: 498 additions & 1 deletion client/dist/styles/bundle.css

Large diffs are not rendered by default.

29 changes: 4 additions & 25 deletions client/src/components/ElementActions/PublishAction.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,19 @@
/* global window */
import React, { useContext, useEffect } from 'react';
import { compose } from 'redux';
import React, { useContext } from 'react';
import AbstractAction from 'components/ElementActions/AbstractAction';
import publishBlockMutation from 'state/editor/publishBlockMutation';
import i18n from 'i18n';
import { ElementContext } from 'components/ElementEditor/Element';
import { ElementContext } from 'components/ElementEditor/ElementContext';

/**
* Adds the elemental menu action to publish a draft/modified block
*/
const PublishAction = (MenuComponent) => (props) => {
const {
doPublishElement,
formDirty,
formHasRendered,
onAfterPublish,
onPublishButtonClick,
} = useContext(ElementContext);

const { element, actions } = props;

const publishElement = () => {
// handlePublishBlock is a graphql mutation defined in publishBlockMutation.js
actions.handlePublishBlock(element.id)
.then(() => onAfterPublish(false))
.catch(() => onAfterPublish(true));
};
const { element } = props;

const handleClick = (event) => {
event.stopPropagation();
Expand All @@ -46,12 +34,6 @@ const PublishAction = (MenuComponent) => (props) => {
toggle: props.toggle,
};

useEffect(() => {
if (formHasRendered && doPublishElement) {
publishElement();
}
}, [formHasRendered, doPublishElement]);

if (props.type.broken) {
// Don't allow this action for a broken element.
return (
Expand All @@ -69,7 +51,4 @@ const PublishAction = (MenuComponent) => (props) => {

export { PublishAction as Component };

export default compose(
publishBlockMutation,
PublishAction
);
export default PublishAction;
19 changes: 2 additions & 17 deletions client/src/components/ElementActions/SaveAction.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import React, { useContext, useEffect } from 'react';
import React, { useContext } from 'react';
import AbstractAction from 'components/ElementActions/AbstractAction';
import i18n from 'i18n';
import { ElementContext } from 'components/ElementEditor/Element';
import { ElementContext } from 'components/ElementEditor/ElementContext';

const SaveAction = (MenuComponent) => (props) => {
const {
doSaveElement,
onSaveButtonClick,
onAfterSave,
submitForm,
formHasRendered,
formDirty,
} = useContext(ElementContext);

Expand All @@ -18,24 +14,13 @@ const SaveAction = (MenuComponent) => (props) => {
onSaveButtonClick();
};

const saveElement = () => {
submitForm();
onAfterSave();
};

const newProps = {
title: i18n._t('ElementSaveAction.SAVE', 'Save'),
className: 'element-editor__actions-save',
onClick: handleClick,
toggle: props.toggle,
};

useEffect(() => {
if (formHasRendered && doSaveElement) {
saveElement();
}
}, [formHasRendered, doSaveElement]);

if (!props.expandable || props.type.broken) {
// Some elemental blocks can not be edited inline (e.g. User form blocks)
// We don't want to add a "Save" action for those blocks.
Expand Down
43 changes: 1 addition & 42 deletions client/src/components/ElementActions/tests/PublishAction-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import React from 'react';
import { fireEvent, render } from '@testing-library/react';
import { Component as PublishAction } from '../PublishAction';
import { ElementContext } from '../../ElementEditor/Element';
import { ElementContext } from '../../ElementEditor/ElementContext';

window.jQuery = {
noticeAdd: () => null
Expand Down Expand Up @@ -101,22 +101,7 @@ test('Clicking button calls onPublishButtonClick', () => {
expect(onPublishButtonClick).toHaveBeenCalled();
});

test('Do trigger graphql mutation if doPublishElement is true and formHasRendered is true', () => {
const handlePublishBlock = jest.fn(() => Promise.resolve());
render(
<ElementContext.Provider value={makeProviderValue({
doPublishElement: true,
formHasRendered: true,
})}
>
<ActionComponent {...makeProps({ actions: { handlePublishBlock } })}/>
</ElementContext.Provider>
);
expect(handlePublishBlock).toHaveBeenCalledWith(123);
});

test('Do not trigger graphql mutation if doPublishElement is true and formHasRendered is false', () => {
// handlePublishBlock is a graphql mutation defined in publishBlockMutation.js
const handlePublishBlock = jest.fn(() => Promise.resolve());
render(
<ElementContext.Provider value={makeProviderValue({
Expand All @@ -143,29 +128,3 @@ test('Do not trigger graphql mutation if doPublishElement is false and formHasRe
);
expect(handlePublishBlock).not.toHaveBeenCalled();
});

test('onAfterPublish is called after graphql mutation', async () => {
let value = 1;
const handlePublishBlock = jest.fn(() => {
value = 2;
return Promise.resolve();
});
const onAfterPublish = jest.fn(() => {
value = 3;
});
render(
<ElementContext.Provider value={makeProviderValue({
doPublishElement: true,
formHasRendered: true,
onAfterPublish,
})}
>
<ActionComponent {...makeProps({ actions: { handlePublishBlock } })}/>
</ElementContext.Provider>
);
// This is required to ensure the resolved promised returned by handlePublishBlock is handled
await new Promise(resolve => setTimeout(resolve, 0));
expect(handlePublishBlock).toHaveBeenCalled();
expect(onAfterPublish).toHaveBeenCalled();
expect(value).toBe(3);
});
75 changes: 1 addition & 74 deletions client/src/components/ElementActions/tests/SaveAction-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import React from 'react';
import { fireEvent, render } from '@testing-library/react';
import { Component as SaveAction } from '../SaveAction';
import { ElementContext } from '../../ElementEditor/Element';
import { ElementContext } from '../../ElementEditor/ElementContext';

function makeProps(obj = {}) {
return {
Expand All @@ -28,11 +28,7 @@ function makeProps(obj = {}) {

function makeProviderValue(obj = {}) {
return {
formHasRendered: false,
onAfterSave: () => {},
doSaveElement: false,
onSaveButtonClick: () => {},
submitForm: () => {},
formDirty: true,
...obj,
};
Expand Down Expand Up @@ -98,72 +94,3 @@ test('Clicking button calls onSaveButtonClick', () => {
fireEvent.click(container.querySelector('button.element-editor__actions-save'));
expect(onSaveButtonClick).toHaveBeenCalled();
});

test('submitForm is called if formHasRendered is true and doSaveElement is true', () => {
const submitForm = jest.fn();
render(
<ElementContext.Provider value={makeProviderValue({
formHasRendered: true,
doSaveElement: true,
submitForm,
})}
>
<ActionComponent {...makeProps()} />
</ElementContext.Provider>
);
expect(submitForm).toHaveBeenCalled();
});

test('submitForm is not called if formHasRendered is false and doSaveElement is true', () => {
const submitForm = jest.fn();
render(
<ElementContext.Provider value={makeProviderValue({
formHasRendered: false,
doSaveElement: true,
submitForm,
})}
>
<ActionComponent {...makeProps()} />
</ElementContext.Provider>
);
expect(submitForm).not.toHaveBeenCalled();
});

test('submitForm is not called if formHasRendered is true and doSaveElement is false', () => {
const submitForm = jest.fn();
render(
<ElementContext.Provider value={makeProviderValue({
formHasRendered: true,
doSaveElement: false,
submitForm,
})}
>
<ActionComponent {...makeProps()} />
</ElementContext.Provider>
);
expect(submitForm).not.toHaveBeenCalled();
});

test('onAfterSave is called after submitForm', () => {
let value = 1;
const submitForm = jest.fn(() => {
value = 2;
});
const onAfterSave = jest.fn(() => {
value = 3;
});
render(
<ElementContext.Provider value={makeProviderValue({
formHasRendered: true,
doSaveElement: true,
submitForm,
onAfterSave,
})}
>
<ActionComponent {...makeProps()}/>
</ElementContext.Provider>
);
expect(submitForm).toHaveBeenCalled();
expect(onAfterSave).toHaveBeenCalled();
expect(value).toBe(3);
});
54 changes: 36 additions & 18 deletions client/src/components/ElementEditor/Content.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class Content extends PureComponent {
onFormSchemaSubmitResponse,
ensureFormRendered,
formHasRendered,
innerRef,
} = this.props;

// The '*-rendered-not-visible` class is used to hide the form when it's not visible
Expand All @@ -36,6 +37,19 @@ class Content extends PureComponent {
'element-editor-editform--rendered-not-visible': notVisible,
};

// console.log([
// 'notVisible', notVisible,
// 'previewExpanded', previewExpanded,
// 'ensureFormRendered', ensureFormRendered,
// 'formHasRendered', formHasRendered,
// 'extraClass', extraClass
// ])
console.log('&& Content.formDirty', formDirty);
console.log('&& Content.ensureFormRendered', ensureFormRendered);
console.log('&& Content.formHasRendered', formHasRendered);
console.log('&& Content.previewExpanded', previewExpanded);
console.log('&& Content.notVisible', notVisible);

return (
<div className="element-editor-content">
{!previewExpanded &&
Expand All @@ -49,16 +63,19 @@ class Content extends PureComponent {
}
{(previewExpanded || ensureFormRendered || formHasRendered) &&
// Show inline editable fields
<InlineEditFormComponent
extraClass={extraClass}
onClick={(event) => event.stopPropagation()}
elementId={id}
activeTab={activeTab}
onFormInit={onFormInit}
handleLoadingError={handleLoadingError}
onFormSchemaSubmitResponse={onFormSchemaSubmitResponse}
notVisible={notVisible}
/>
<>
<InlineEditFormComponent
extraClass={extraClass}
onClick={(event) => event.stopPropagation()}
elementId={id}
activeTab={activeTab}
onFormInit={onFormInit}
handleLoadingError={handleLoadingError}
onFormSchemaSubmitResponse={onFormSchemaSubmitResponse}
notVisible={notVisible}
/>
<div ref={innerRef}></div>
</>
}
{formDirty &&
<input
Expand Down Expand Up @@ -91,13 +108,13 @@ Content.propTypes = {

Content.defaultProps = {};

function mapStateToProps(state, ownProps) {
const formName = loadElementFormStateName(ownProps.id);
// function mapStateToProps(state, ownProps) {
// const formName = loadElementFormStateName(ownProps.id);

return {
formDirty: isDirty(`element.${formName}`, getFormState)(state),
};
}
// return {
// formDirty: isDirty(`element.${formName}`, getFormState)(state),
// };
// }

export { Content as Component };

Expand All @@ -108,6 +125,7 @@ export default compose(
SummaryComponent, InlineEditFormComponent,
}),
() => 'ElementEditor.ElementList.Element'
),
connect(mapStateToProps)
)
// ,
// connect(mapStateToProps)
)(Content);
Loading
Loading