Skip to content

Commit

Permalink
refactor: small cleanup around fullscreenmodal
Browse files Browse the repository at this point in the history 
removing components that just add classes, so that we can replace them with
their ui-kit pendants some time soon.

pulling out some static content (it's easier to reason about components if there
can't be a `this` involved).

get rid of `.modal-full-screen-header-with-sub-title`, which was aligning the
height of the header when installing Frameworks. chose to remove all the
complexity around it and live with a headline that's off by 2 pixels to replace
it with a ui-kit component in the future.

replace PropTypes with TS-typings.
  • Loading branch information
@pierrebeitz
pierrebeitz committed Aug 24, 2020
1 parent b916fe4 commit f5f99d9
Show file tree
Hide file tree
Showing 12 changed files with 78 additions and 213 deletions.
4 changes: 2 additions & 2 deletions plugins/jobs/src/js/components/JobsFormModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import isEqual from "lodash/isEqual";
import FullScreenModal from "#SRC/js/components/modals/FullScreenModal";
import FullScreenModalHeader from "#SRC/js/components/modals/FullScreenModalHeader";
import FullScreenModalHeaderActions from "#SRC/js/components/modals/FullScreenModalHeaderActions";
import FullScreenModalHeaderTitle from "#SRC/js/components/modals/FullScreenModalHeaderTitle";
import DataValidatorUtil from "#SRC/js/utils/DataValidatorUtil";
import ModalHeading from "#SRC/js/components/modals/ModalHeading";
import ToggleButton from "#SRC/js/components/ToggleButton";
Expand Down Expand Up @@ -403,7 +402,8 @@ class JobFormModal extends React.Component<
actions={this.getSecondaryActions()}
type="secondary"
/>
<FullScreenModalHeaderTitle>{title}</FullScreenModalHeaderTitle>

<div className="modal-full-screen-header-title">{title}</div>
<FullScreenModalHeaderActions
actions={this.getPrimaryActions()}
type="primary"
Expand Down
88 changes: 41 additions & 47 deletions plugins/jobs/src/js/components/form/GeneralFormSection.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,35 +22,37 @@ interface GeneralProps {
isEdit: boolean;
}

function getFieldError(path: string, errors: FormError[]) {
return errors
.filter((e) => {
const match = e.path.join(".");
return match === path;
})
const getFieldError = (path: string, errors: FormError[]) =>
errors
.filter((e) => e.path.join(".") === path)
.map((e) => e.message)
.join(" ");
}

class GeneralFormSection extends React.Component<GeneralProps> {
constructor(props: Readonly<GeneralProps>) {
super(props);
}
const idTooltipContent = (
<Trans>
Unique identifier for the job consisting of a series of names separated by
dots. Each name must be at least 1 character and may only contain digits
(`0-9`), dashes (`-`), and lowercase letters (`a-z`). The name may not begin
or end with a dash.
</Trans>
);
const descTooltipContent = <Trans>A description of this job.</Trans>;
const cpuTooltipContent = (
<Trans>
The number of CPU shares this job needs per instance. This number does not
have to be integer, but can be a fraction.
</Trans>
);
const gpuTooltipContent = (
<Trans>
The number of GPU shares this job needs per instance. Only available with
the UCR runtime.
</Trans>
);

export default class GeneralFormSection extends React.Component<GeneralProps> {
public getResourceRow() {
const { formData, showErrors, errors } = this.props;
const cpuTooltipContent = (
<Trans>
The number of CPU shares this job needs per instance. This number does
not have to be integer, but can be a fraction.
</Trans>
);
const gpuTooltipContent = (
<Trans>
The number of GPU shares this job needs per instance. Only available
with the UCR runtime.
</Trans>
);
const gpusDisabled = !formData.cmdOnly && formData.container !== "ucr";

const cpusError = getFieldError("run.cpus", errors);
Expand All @@ -60,7 +62,7 @@ class GeneralFormSection extends React.Component<GeneralProps> {

return (
<FormRow>
<FormGroup className="column-3" showError={showErrors && cpusError}>
<FormGroup className="column-3" showError={showErrors && !!cpusError}>
<FieldLabel>
<FormGroupHeading required={true}>
<FormGroupHeadingContent title="CPUs" primary={true}>
Expand All @@ -82,12 +84,12 @@ class GeneralFormSection extends React.Component<GeneralProps> {
type="number"
name="job.run.cpus"
value={formData.cpus}
autoFocus={showErrors && cpusError}
autoFocus={showErrors && !!cpusError}
/>
<FieldError>{cpusError}</FieldError>
</FormGroup>

<FormGroup className="column-3" showError={showErrors && memError}>
<FormGroup className="column-3" showError={showErrors && !!memError}>
<FieldLabel className="text-no-transform">
<FormGroupHeading required={true}>
<FormGroupHeadingContent title="Mem">
Expand All @@ -99,12 +101,12 @@ class GeneralFormSection extends React.Component<GeneralProps> {
name="job.run.mem"
type="number"
value={formData.mem}
autoFocus={showErrors && memError}
autoFocus={showErrors && !!memError}
/>
<FieldError>{memError}</FieldError>
</FormGroup>

<FormGroup className="column-3" showError={showErrors && diskError}>
<FormGroup className="column-3" showError={showErrors && !!diskError}>
<FieldLabel className="text-no-transform">
<FormGroupHeading required={true}>
<FormGroupHeadingContent title="Disk">
Expand All @@ -116,12 +118,12 @@ class GeneralFormSection extends React.Component<GeneralProps> {
name="job.run.disk"
type="number"
value={formData.disk}
autoFocus={showErrors && diskError}
autoFocus={showErrors && !!diskError}
/>
<FieldError>{diskError}</FieldError>
</FormGroup>

<FormGroup className="column-3" showError={showErrors && gpusError}>
<FormGroup className="column-3" showError={showErrors && !!gpusError}>
<FieldLabel className="text-no-transform">
<FormGroupHeading>
<FormGroupHeadingContent title="GPUs">
Expand All @@ -144,7 +146,7 @@ class GeneralFormSection extends React.Component<GeneralProps> {
type="number"
disabled={gpusDisabled}
value={formData.gpus}
autoFocus={showErrors && gpusError}
autoFocus={showErrors && !!gpusError}
/>
<FieldError>{gpusError}</FieldError>
</FormGroup>
Expand Down Expand Up @@ -185,7 +187,7 @@ class GeneralFormSection extends React.Component<GeneralProps> {
checked={!formData.cmdOnly}
name="cmdOnly"
type="radio"
value={false}
value={"false"}
/>
<Trans render="span">Container Image</Trans>
</FieldLabel>
Expand All @@ -194,7 +196,7 @@ class GeneralFormSection extends React.Component<GeneralProps> {
checked={formData.cmdOnly}
name="cmdOnly"
type="radio"
value={true}
value={"true"}
/>
<Trans render="span">Command Only</Trans>
</FieldLabel>
Expand All @@ -203,7 +205,7 @@ class GeneralFormSection extends React.Component<GeneralProps> {
<FormRow>
<FormGroup
className="column-12"
showError={showErrors && containerImageErrors}
showError={showErrors && !!containerImageErrors}
>
<FieldLabel>
<FormGroupHeading required={!formData.cmdOnly}>
Expand Down Expand Up @@ -242,7 +244,10 @@ class GeneralFormSection extends React.Component<GeneralProps> {
</FormRow>
)}
<FormRow>
<FormGroup className="column-12" showError={showErrors && cmdErrors}>
<FormGroup
className="column-12"
showError={showErrors && !!cmdErrors}
>
<FieldLabel>
<FormGroupHeading required={formData.cmdOnly}>
<FormGroupHeadingContent title="Command" primary={true}>
Expand Down Expand Up @@ -282,15 +287,6 @@ class GeneralFormSection extends React.Component<GeneralProps> {
public render() {
const { formData, errors, showErrors, isEdit } = this.props;

const idTooltipContent = (
<Trans>
Unique identifier for the job consisting of a series of names separated
by dots. Each name must be at least 1 character and may only contain
digits (`0-9`), dashes (`-`), and lowercase letters (`a-z`). The name
may not begin or end with a dash.
</Trans>
);
const descTooltipContent = <Trans>A description of this job.</Trans>;
const idError = getFieldError("id", errors);

return (
Expand All @@ -299,7 +295,7 @@ class GeneralFormSection extends React.Component<GeneralProps> {
General
</Trans>
<FormRow>
<FormGroup className="column-12" showError={showErrors && idError}>
<FormGroup className="column-12" showError={showErrors && !!idError}>
<FieldLabel>
<FormGroupHeading required={true}>
<FormGroupHeadingContent title="Job ID" primary={true}>
Expand Down Expand Up @@ -362,5 +358,3 @@ class GeneralFormSection extends React.Component<GeneralProps> {
);
}
}

export default GeneralFormSection;
9 changes: 4 additions & 5 deletions plugins/services/src/js/components/modals/CreateServiceModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import DataValidatorUtil from "#SRC/js/utils/DataValidatorUtil";
import FullScreenModal from "#SRC/js/components/modals/FullScreenModal";
import FullScreenModalHeader from "#SRC/js/components/modals/FullScreenModalHeader";
import FullScreenModalHeaderActions from "#SRC/js/components/modals/FullScreenModalHeaderActions";
import FullScreenModalHeaderTitle from "#SRC/js/components/modals/FullScreenModalHeaderTitle";
import ModalHeading from "#SRC/js/components/modals/ModalHeading";
import PodValidators from "#SRC/resources/raml/marathon/v2/types/pod.raml";
import ToggleButton from "#SRC/js/components/ToggleButton";
Expand Down Expand Up @@ -554,9 +553,9 @@ class CreateServiceModal extends React.Component {
actions={this.getSecondaryActions()}
type="secondary"
/>
<FullScreenModalHeaderTitle>
<div className="modal-full-screen-header-title">
<Trans render="span">Review & Run Service</Trans>
</FullScreenModalHeaderTitle>
</div>
<FullScreenModalHeaderActions
actions={this.getPrimaryActions()}
type="primary"
Expand All @@ -583,7 +582,7 @@ class CreateServiceModal extends React.Component {
actions={this.getSecondaryActions()}
type="secondary"
/>
<FullScreenModalHeaderTitle>{title}</FullScreenModalHeaderTitle>
<div className="modal-full-screen-header-title">{title}</div>
<FullScreenModalHeaderActions
actions={this.getPrimaryActions()}
type="primary"
Expand Down Expand Up @@ -783,7 +782,7 @@ class CreateServiceModal extends React.Component {
}

if (servicePickerActive) {
return null;
return [];
}

if (serviceFormActive) {
Expand Down
5 changes: 2 additions & 3 deletions plugins/services/src/js/components/modals/ServiceRootGroupModal/Header.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { I18n, i18nMark } from "@lingui/core";
import { Trans } from "@lingui/macro";
import FullScreenModalHeader from "#SRC/js/components/modals/FullScreenModalHeader";
import FullScreenModalHeaderActions from "#SRC/js/components/modals/FullScreenModalHeaderActions";
import FullScreenModalHeaderTitle from "#SRC/js/components/modals/FullScreenModalHeaderTitle";

interface GroupModalHeaderProps {
i18n: I18n;
Expand Down Expand Up @@ -39,13 +38,13 @@ export default (props: GroupModalHeaderProps) => {
]}
type="secondary"
/>
<FullScreenModalHeaderTitle>
<div className="modal-full-screen-header-title">
{mode === "create" ? (
<Trans>New Group</Trans>
) : (
<Trans>Edit Group</Trans>
)}
</FullScreenModalHeaderTitle>
</div>
<FullScreenModalHeaderActions
actions={[
{
Expand Down
22 changes: 0 additions & 22 deletions src/js/__tests__/__snapshots__/typecheck-test.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,19 +377,6 @@ plugins/jobs/src/js/components/form/ContainerFormSection.tsx: error TS2322: type
plugins/jobs/src/js/components/form/EnvVarPartial.tsx: error TS2741: Property 'onClick' is missing in type '{}' but required in type *.
plugins/jobs/src/js/components/form/EnvVarPartial.tsx: error TS2322: Type 'void' is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: Type 'false' is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: Type 'true' is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/GeneralFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/jobs/src/js/components/form/ParametersSection.tsx: error TS2741: Property 'onClick' is missing in type '{}' but required in type *.
plugins/jobs/src/js/components/form/ParametersSection.tsx: error TS2322: Type 'void' is not assignable to type *.
plugins/jobs/src/js/components/form/PlacementPartial.tsx: error TS2322: Type 'void' is not assignable to type *.
Expand Down Expand Up @@ -1594,7 +1581,6 @@ plugins/organization/submodules/service-accounts/components/ServiceAccountDetail
plugins/organization/submodules/service-accounts/components/ServiceAccountDetailPage.tsx: error TS2339: Property 'fetchedDetailsError' does not exist on type 'Readonly<{}>'.
plugins/organization/submodules/service-accounts/components/ServiceAccountDetailPage.tsx: error TS2339: Property 'isRemote' does not exist on type 'ServiceAccount'.
plugins/organization/submodules/service-accounts/components/ServiceAccountDetailPage.tsx: error TS2339: Property 'isRemote' does not exist on type 'ServiceAccount'.
plugins/organization/submodules/service-accounts/components/ServiceAccountForm.tsx: error TS2322: type * is not assignable to type *.
plugins/organization/submodules/service-accounts/components/ServiceAccountsActionsModal.tsx: error TS2339: Property 'store_listeners' does not exist on type 'ServiceAccountsActionsModal'.
plugins/organization/submodules/service-accounts/components/ServiceAccountsActionsModal.tsx: error TS2339: Property 'store_listeners' does not exist on type 'ServiceAccountsActionsModal'.
plugins/organization/submodules/service-accounts/components/ServiceAccountsActionsModal.tsx: error TS2556: Expected 0 arguments, but got 1 or more.
Expand Down Expand Up @@ -1854,13 +1840,11 @@ plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'localErr
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'localErrors' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'textValue' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'localErrors' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2322: type * is not assignable to type *.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'fileValue' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'localErrors' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'localErrors' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2322: type * is not assignable to type *.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'localErrors' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2322: type * is not assignable to type *.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'path' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'textValue' does not exist on type 'Readonly<{}>'.
plugins/secrets/components/SecretFormModal.tsx: error TS2339: Property 'fileValue' does not exist on type 'Readonly<{}>'.
Expand Down Expand Up @@ -2424,7 +2408,6 @@ plugins/services/src/js/components/forms/ContainerServiceFormSection.tsx: error
plugins/services/src/js/components/forms/ContainerServiceFormSection.tsx: error TS2339: Property 'data' does not exist on type *.
plugins/services/src/js/components/forms/ContainerServiceFormSection.tsx: error TS2339: Property 'errors' does not exist on type *.
plugins/services/src/js/components/forms/ContainerServiceFormSection.tsx: error TS2339: Property 'path' does not exist on type *.
plugins/services/src/js/components/forms/ContainerServiceFormSection.tsx: error TS2322: type * is not assignable to type *.
plugins/services/src/js/components/forms/ContainerServiceFormSection.tsx: error TS2339: Property 'configReducers' does not exist on type *.
plugins/services/src/js/components/forms/ContainerServiceFormSection.tsx: error TS2339: Property 'service' does not exist on type *.
plugins/services/src/js/components/forms/EnvironmentFormSection.tsx: error TS2339: Property 'onRemoveItem' does not exist on type *.
Expand Down Expand Up @@ -5547,11 +5530,6 @@ src/js/components/modals/ActionsModal.tsx: error TS2339: Property 'itemType' doe
src/js/components/modals/ActionsModal.tsx: error TS2339: Property 'requestErrors' does not exist on type 'Readonly<{}>'.
src/js/components/modals/ActionsModal.tsx: error TS2339: Property 'validationError' does not exist on type 'Readonly<{}>'.
src/js/components/modals/FullScreenModal.tsx: error TS2322: Type '\\"100%\\"' is not assignable to type 'null'.
src/js/components/modals/FullScreenModalHeaderActions.tsx: error TS2448: Block-scoped variable 'classProps' used before its declaration.
src/js/components/modals/FullScreenModalHeaderActions.tsx: error TS2448: Block-scoped variable 'classProps' used before its declaration.
src/js/components/modals/FullScreenModalHeaderActions.tsx: error TS2339: Property 'actions' does not exist on type *.
src/js/components/modals/FullScreenModalHeaderActions.tsx: error TS2339: Property 'className' does not exist on type *.
src/js/components/modals/FullScreenModalHeaderActions.tsx: error TS2339: Property 'type' does not exist on type *.
src/js/components/modals/ImageViewerModal.tsx: error TS2339: Property 'isLoadingImage' does not exist on type 'Readonly<{}>'.
src/js/components/modals/ImageViewerModal.tsx: error TS2339: Property 'onClose' does not exist on type *.
src/js/components/modals/ImageViewerModal.tsx: error TS2339: Property 'open' does not exist on type *.
Expand Down
Loading

0 comments on commit f5f99d9

Please sign in to comment.