Skip to content

Commit

Permalink
[#198] the yaml editor allows to enter new Lines
Browse files Browse the repository at this point in the history
Previously the yaml editor wasn't allowing a user to add new lines. This
was because at each keystroke the yaml file got formatted automatically,
removing what formatting felt as unnecessary lines.. This meant that the
user had to use tricks like copy&pasting to get to a state the wanted to
have.

To fix that, this commits reverts back to using the resource editor
provided by the console. It has several advantages:
* The use can edit the file as they want without bugs
* There's documentation included from the CRDs when hovering properties

The main drawback is that the user needs to click on save to save their
changes to the model, then needs to click on apply to apply their
config (either creating or updating)

Other additions:
----------------

In order to make the logic of the buttons the same between the two
views, they have been extracted up a notch in the component hierarchy.

Warning modals have been added to help the user know if they are about
to lose some changes.

A reload button was added to the update broker page, so that the user
can reload the content of the existing broker.
  • Loading branch information
lavocatt committed Sep 10, 2024
1 parent ab563a2 commit 146eb2b
Show file tree
Hide file tree
Showing 12 changed files with 719 additions and 289 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
"react-router-dom-v5-compat": "6.22.3",
"sort-package-json": "^2.10.0",
"style-loader": "^3.3.1",
"css-loader": "^6.7.1",
"stylelint": "^15.3.0",
"stylelint-config-prettier": "9.0.3",
"stylelint-config-standard": "^31.0.0",
Expand Down
129 changes: 129 additions & 0 deletions src/brokers/add-broker/AddBroker.component.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { FC, useReducer } from 'react';
import {
BrokerCreationFormDispatch,
BrokerCreationFormState,
artemisCrReducer,
newArtemisCRState,
} from '../../reducers/7.12/reducer';
import { fireEvent, render, screen } from '../../test-utils';
import { AddBroker } from './AddBroker.component';
import { useAccessReview } from '@openshift-console/dynamic-plugin-sdk';

jest.mock('@openshift-console/dynamic-plugin-sdk', () => ({
useAccessReview: jest.fn(() => []),
}));

const mockUseAccessReview = useAccessReview as jest.Mock;

const SimplifiedCreaterBrokerPage: FC = () => {
const onSubmit = () => {
return 0;
};
const onCancel = () => {
return 0;
};
const initialValues = newArtemisCRState('default');
const [brokerModel, dispatch] = useReducer(artemisCrReducer, initialValues);
return (
<BrokerCreationFormState.Provider value={brokerModel}>
<BrokerCreationFormDispatch.Provider value={dispatch}>
<AddBroker onSubmit={onSubmit} onCancel={onCancel} />
</BrokerCreationFormDispatch.Provider>
</BrokerCreationFormState.Provider>
);
};

const SimplifiedUpdateBrokerPage: FC = () => {
const onSubmit = () => {
return 0;
};
const onCancel = () => {
return 0;
};
const reloadExisting = () => {
return 0;
};
const initialValues = newArtemisCRState('default');
const [brokerModel, dispatch] = useReducer(artemisCrReducer, initialValues);
return (
<BrokerCreationFormState.Provider value={brokerModel}>
<BrokerCreationFormDispatch.Provider value={dispatch}>
<AddBroker
onSubmit={onSubmit}
onCancel={onCancel}
isUpdatingExisting
reloadExisting={reloadExisting}
/>
</BrokerCreationFormDispatch.Provider>
</BrokerCreationFormState.Provider>
);
};

describe('create broker', () => {
beforeEach(() => {
jest.clearAllMocks();

mockUseAccessReview.mockReturnValue([true, false]);
});
it('clicking on cancel after making some changes displays a warning', async () => {
render(<SimplifiedCreaterBrokerPage />);
fireEvent.click(screen.getByRole('button', { name: /plus/i }));
fireEvent.click(screen.getByRole('button', { name: /cancel/i }));
expect(
screen.getByText(
"You are about to quit the editor, the broker won't get created",
),
).toBeInTheDocument();
});
it("clicking on cancel immediately after opening the editor shouldn't display a warning", async () => {
render(<SimplifiedCreaterBrokerPage />);
fireEvent.click(screen.getByRole('button', { name: /cancel/i }));
expect(
screen.queryByText(
"You are about to quit the editor, the broker won't get created",
),
).not.toBeInTheDocument();
});
});

describe('update broker', () => {
beforeEach(() => {
jest.clearAllMocks();

mockUseAccessReview.mockReturnValue([true, false]);
});
it('clicking on cancel after making some changes displays a warning', async () => {
render(<SimplifiedUpdateBrokerPage />);
fireEvent.click(screen.getByRole('button', { name: /plus/i }));
fireEvent.click(screen.getByRole('button', { name: /cancel/i }));
expect(
screen.queryByText(
'You are about to quit the editor, configuration that is not applied will be lost',
),
).toBeInTheDocument();
});
it("clicking on cancel immediately after opening the editor shouldn't display a warning", async () => {
render(<SimplifiedUpdateBrokerPage />);
fireEvent.click(screen.getByRole('button', { name: /cancel/i }));
expect(
screen.queryByText(
'You are about to quit the editor, configuration that is not applied will be lost',
),
).not.toBeInTheDocument();
});
it('clicking on reload after making some changes displays a warning', async () => {
render(<SimplifiedUpdateBrokerPage />);
fireEvent.click(screen.getByRole('button', { name: /plus/i }));
fireEvent.click(screen.getByRole('button', { name: /reload/i }));
expect(
screen.queryByText('Upon reloading, these modifications will be lost.'),
).toBeInTheDocument();
});
it("clicking on reload immediately after opening the editor shouldn't display a warning", async () => {
render(<SimplifiedUpdateBrokerPage />);
fireEvent.click(screen.getByRole('button', { name: /reload/i }));
expect(
screen.queryByText('Upon reloading, these modifications will be lost.'),
).not.toBeInTheDocument();
});
});
216 changes: 180 additions & 36 deletions src/brokers/add-broker/AddBroker.component.tsx
Original file line number Diff line number Diff line change
@@ -1,68 +1,212 @@
import { FC, useContext } from 'react';
import { K8sResourceCommon } from '@openshift-console/dynamic-plugin-sdk';
import { AlertVariant, Divider } from '@patternfly/react-core';
import { FC, useContext, useState } from 'react';
import {
ActionGroup,
Alert,
AlertVariant,
Button,
ButtonVariant,
Divider,
Form,
FormFieldGroup,
Modal,
ModalVariant,
} from '@patternfly/react-core';
import {
ArtemisReducerOperations,
BrokerCreationFormDispatch,
BrokerCreationFormState,
EditorType,
} from '../../reducers/7.12/reducer';
import { useLocation } from 'react-router-dom-v5-compat';
import { FormView } from '../../shared-components/FormView/FormView';
import { YamlEditorView } from '../../shared-components/YamlEditorView/YamlEditorView';
import { EditorToggle } from './components/EditorToggle/EditorToggle';
import { Loading } from '../../shared-components/Loading/Loading';
import { useAccessReview } from '@openshift-console/dynamic-plugin-sdk';
import { AMQBrokerModel } from '../../k8s/models';
import { useTranslation } from '../../i18n/i18n';
import { YamlEditorView } from '../../shared-components/YamlEditorView/YamlEditorView';

type AddBrokerProps = {
onCreateBroker: (data?: K8sResourceCommon) => void;
notification: {
title: string;
variant: AlertVariant;
};
isUpdate?: boolean;
type AddBrokerPropTypes = {
onSubmit: () => void;
onCancel: () => void;
isUpdatingExisting?: boolean;
reloadExisting?: () => void;
};

export const AddBroker: FC<AddBrokerProps> = ({
onCreateBroker,
notification,
isUpdate,
export const AddBroker: FC<AddBrokerPropTypes> = ({
onSubmit,
onCancel: doQuit,
isUpdatingExisting,
reloadExisting,
}) => {
const formValues = useContext(BrokerCreationFormState);
const dispatch = useContext(BrokerCreationFormDispatch);
const location = useLocation();
const params = new URLSearchParams(location.search);
const returnUrl = params.get('returnUrl') || '/k8s/all-namespaces/brokers';

const { editorType } = formValues;

const [userWantsToQuit, setUserWantsToQuit] = useState<boolean>(false);
const [userWantsToReload, setUserWantsToReload] = useState<boolean>(false);

const [pendingActionQuittingYAMLView, setPendingActionQuittingYAMLView] =
useState<'switch' | 'submit'>('switch');
const [wantsToQuitYamlView, setWantsToQuitYamlView] = useState(false);
const onSelectEditorType = (editorType: EditorType) => {
dispatch({
operation: ArtemisReducerOperations.setEditorType,
payload: editorType,
});
if (formValues.editorType === EditorType.YAML) {
if (editorType === EditorType.BROKER) {
setWantsToQuitYamlView(true);
}
setPendingActionQuittingYAMLView('switch');
} else {
dispatch({
operation: ArtemisReducerOperations.setEditorType,
payload: EditorType.YAML,
});
}
};
const [triggerDelayedSubmit, setTriggerDelayedSubmit] = useState(false);
const [prevTriggerDelayedSubmit, setPrevTriggerDelayedSubmit] =
useState(triggerDelayedSubmit);
const onQuittingYamlView = () => {
if (pendingActionQuittingYAMLView === 'switch') {
setWantsToQuitYamlView(false);
dispatch({
operation: ArtemisReducerOperations.setEditorType,
payload: EditorType.BROKER,
});
}
if (pendingActionQuittingYAMLView === 'submit') {
setWantsToQuitYamlView(false);
setTriggerDelayedSubmit(true);
}
};
if (triggerDelayedSubmit !== prevTriggerDelayedSubmit) {
if (triggerDelayedSubmit) {
setTriggerDelayedSubmit(false);
onSubmit();
}
setPrevTriggerDelayedSubmit(triggerDelayedSubmit);
}

const { t } = useTranslation();
const namespace = formValues.cr.metadata.namespace;
const [canCreateBroker, loadingAccessReview] = useAccessReview({
group: AMQBrokerModel.apiGroup,
resource: AMQBrokerModel.plural,
namespace,
verb: 'create',
});
if (loadingAccessReview) return <Loading />;
if (!canCreateBroker) {
return (
<Alert
variant={AlertVariant.custom}
title={t('broker_can_not_be_created')}
isInline
>
{t('you_do_not_have_write_access')}
</Alert>
);
}
return (
<>
<Modal
variant={ModalVariant.small}
title="Unsaved changes"
isOpen={userWantsToQuit}
onClose={() => setUserWantsToQuit(false)}
actions={[
<Button key="confirm" variant="primary" onClick={doQuit}>
Confirm
</Button>,
<Button
key="cancel"
variant="link"
onClick={() => setUserWantsToQuit(false)}
>
Cancel
</Button>,
]}
>
You are about to quit the editor,{' '}
{isUpdatingExisting
? 'configuration that is not applied will be lost'
: "the broker won't get created"}
</Modal>
<Modal
variant={ModalVariant.small}
title="Unsaved changes"
isOpen={userWantsToReload}
onClose={() => setUserWantsToReload(false)}
actions={[
<Button key="confirm" variant="primary" onClick={reloadExisting}>
Confirm
</Button>,
<Button
key="cancel"
variant="link"
onClick={() => setUserWantsToReload(false)}
>
Cancel
</Button>,
]}
>
Upon reloading, these modifications will be lost.
</Modal>
<Divider />
<EditorToggle value={editorType} onChange={onSelectEditorType} />
<Divider />
{editorType === EditorType.BROKER && (
<FormView
onCreateBroker={onCreateBroker}
notification={notification}
isUpdate={isUpdate}
returnUrl={returnUrl}
/>
)}
{editorType === EditorType.BROKER && <FormView />}
{editorType === EditorType.YAML && (
<YamlEditorView
onCreateBroker={onCreateBroker}
initialResourceYAML={formValues.cr}
notification={notification}
isUpdate={isUpdate}
returnUrl={returnUrl}
isAskingPermissionToClose={wantsToQuitYamlView}
permissionGranted={onQuittingYamlView}
permissionDenied={() => setWantsToQuitYamlView(false)}
/>
)}
<Form>
<FormFieldGroup>
<ActionGroup>
<Button
variant={ButtonVariant.primary}
onClick={() => {
if (formValues.editorType === EditorType.YAML) {
setWantsToQuitYamlView(true);
setPendingActionQuittingYAMLView('submit');
} else {
onSubmit();
}
}}
>
{isUpdatingExisting ? 'Apply' : 'Create'}
</Button>
{isUpdatingExisting && (
<Button
variant={ButtonVariant.secondary}
onClick={() => {
if (formValues.hasChanges) {
setUserWantsToReload(true);
} else {
reloadExisting();
}
}}
>
Reload
</Button>
)}
<Button
variant={ButtonVariant.link}
onClick={() => {
if (formValues.hasChanges) {
setUserWantsToQuit(true);
} else {
doQuit();
}
}}
>
Cancel
</Button>
</ActionGroup>
</FormFieldGroup>
</Form>
</>
);
};
Loading

0 comments on commit 146eb2b

Please sign in to comment.