[release-4.5] Bug 1861433: Show descriptions for all dynamic form fields #6136

TheRealJon · 2020-07-28T15:41:34Z

Manual cherry pick of #6054

Manual cherry pick of openshift#6054

openshift-ci-robot · 2020-07-28T15:41:39Z

@TheRealJon: This pull request references Bugzilla bug 1861433, which is invalid:

expected the bug to target the "4.5.z" release, but it targets "4.6.0" instead
expected dependent Bugzilla bug 1854198 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1861433: Show descriptions for all dynamic form fields

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

TheRealJon · 2020-07-28T15:42:35Z

/bugzilla refresh

openshift-ci-robot · 2020-07-28T15:42:38Z

@TheRealJon: This pull request references Bugzilla bug 1861433, which is invalid:

expected dependent Bugzilla bug 1854198 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

spadgett · 2020-07-28T16:05:47Z

@TheRealJon Were there just a few small merge conflicts or did you have to rewrite this for 4.5? Can you point me to where the changes from 4.6 are?

TheRealJon · 2020-07-29T16:01:59Z

There were not many conflicts. The high line count for changes came from moving some components around mainly. I'll leave some comments where the main changes were made.

TheRealJon · 2020-07-29T16:07:44Z

frontend/packages/console-shared/src/components/dynamic-form/fields.tsx

 import * as _ from 'lodash';
+import * as classnames from 'classnames';
 import * as React from 'react';
-import { FieldProps } from 'react-jsonschema-form';
+import { JSONSchema6 } from 'json-schema';
+import { getUiOptions } from 'react-jsonschema-form/lib/utils';
+import { FieldProps, UiSchema } from 'react-jsonschema-form';
+import SchemaField, {
+  SchemaFieldProps,
+} from 'react-jsonschema-form/lib/components/fields/SchemaField';
 import { LinkifyExternal, SelectorInput, Dropdown } from '@console/internal/components/utils';
+import { AccordionContent, AccordionItem, AccordionToggle, Switch } from '@patternfly/react-core';
+import { MatchExpressions } from '@console/operator-lifecycle-manager/src/components/descriptors/spec/match-expressions';
 import { ResourceRequirements } from '@console/operator-lifecycle-manager/src/components/descriptors/spec/resource-requirements';
-import { FieldSet, FormField } from './templates';
-import { ConfigureUpdateStrategy } from '@console/internal/components/modals/configure-update-strategy-modal';
+import {
+  ConfigureUpdateStrategy,
+  UPDATE_STRATEGY_DESCRIPTION,
+} from '@console/internal/components/modals/configure-update-strategy-modal';
 import {
  NodeAffinity,
  PodAffinity,
 } from '@console/operator-lifecycle-manager/src/components/descriptors/spec/affinity';
-import { MatchExpressions } from '@console/operator-lifecycle-manager/src/components/descriptors/spec/match-expressions';
-import { getUiOptions } from 'react-jsonschema-form/lib/utils';
-import { Switch } from '@patternfly/react-core';
-import SchemaField, {
-  SchemaFieldProps,
-} from 'react-jsonschema-form/lib/components/fields/SchemaField';
-import { getSchemaErrors } from './utils';
+import { getSchemaErrors, useSchemaDescription, useSchemaLabel } from './utils';


Almost all of this was from moving components around and adjusting types.

TheRealJon · 2020-07-29T16:07:47Z

frontend/packages/console-shared/src/components/dynamic-form/fields.tsx


-export const DescriptionField: React.FC<FieldProps> = ({ id, description }) =>
+const Description = ({ id, description }) =>


Pulled this out so that it can be used in components that don't have all FieldProps.

TheRealJon · 2020-07-29T16:10:58Z

frontend/packages/console-shared/src/components/dynamic-form/fields.tsx

+  <Description id={id} description={description} />
+);
+
+export const FormField: React.FC<FormFieldProps> = ({


Moved FormField component from templates file and use instead of FieldWrapper component since these two were very similar components. Moving it here was mostly to prevent dependency cycles.

Also refined the props for this component so that it's more easily reused between field and template components.

TheRealJon · 2020-07-29T16:12:11Z

frontend/packages/console-shared/src/components/dynamic-form/fields.tsx

-        </dd>
-      </dl>
-    </FieldSet>
+export const FieldSet: React.FC<FieldSetProps> = ({


Move FieldSet component from templates file to prevent dependency cycle.

Same as FormField component. Refined props so that they are more easily reused or calculated in field or template components.

TheRealJon · 2020-07-29T16:20:31Z

frontend/packages/console-shared/src/components/dynamic-form/fields.tsx

  );
 };

-export const UpdateStrategyField: React.FC<FieldProps> = (props) => {
-  const { formData = {}, idSchema, onChange } = props;
+export const ResourceRequirementsField: React.FC<FieldProps> = ({


This just got pushed down because of the other components that were added at the top of the file. The rest of the changes in this file were mostly just getting rid of the spread operator where FieldSet is used and making better use of the null coalescence operator.

TheRealJon · 2020-07-29T16:27:37Z

frontend/packages/console-shared/src/components/dynamic-form/templates.tsx

+    <FieldSet
+      defaultLabel={title}
+      idSchema={idSchema}
+      required={required}
+      schema={schema}
+      uiSchema={uiSchema}
+    >
      <div className="co-dynamic-form__field-group-content">


New props for FieldSet and getting rid of spread operator

TheRealJon · 2020-07-29T16:34:24Z

frontend/packages/console-shared/src/components/dynamic-form/templates.tsx

  return (
-    <FieldSet {...props}>
-      {_.map(items, (item) => {
+    <FieldSet


New props for FieldSet

TheRealJon · 2020-07-29T16:34:59Z

frontend/packages/console-shared/src/components/dynamic-form/templates.tsx

-import * as classnames from 'classnames';
 import * as React from 'react';
 import {
  ArrayFieldTemplateProps,
  FieldTemplateProps,
  ObjectFieldTemplateProps,
-  FieldProps,
 } from 'react-jsonschema-form';
-import {
-  AccordionItem,
-  AccordionToggle,
-  AccordionContent,
-  Button,
-  Alert,
-} from '@patternfly/react-core';
+import { Button, Alert, FormHelperText } from '@patternfly/react-core';
 import { MinusCircleIcon, PlusCircleIcon } from '@patternfly/react-icons';
 import { JSON_SCHEMA_GROUP_TYPES } from './const';
 import { getUiOptions, getSchemaType } from 'react-jsonschema-form/lib/utils';
 import { ExpandCollapse } from '@console/internal/components/utils';
-
-export const FieldLabel: React.FC<FieldLabelProps> = ({ htmlFor, label, required }) => (
-  <label className={classnames('form-label', { 'co-required': required })} htmlFor={htmlFor}>
-    {label}
-  </label>
-);
-
-export const FormField: React.FC<FormFieldProps> = ({
-  children,
-  displayTitle = true,
-  id,
-  required,
-  title,
-}) => {
-  return (
-    <div id={`${id}_field`} className="form-group">
-      {displayTitle && <FieldLabel label={title || 'Value'} required={required} htmlFor={id} />}
-      {children}
-    </div>
-  );


This is just from moving FieldSet and FormField components to the fields.tsx file.

TheRealJon · 2020-07-29T16:35:03Z

frontend/packages/console-shared/src/components/dynamic-form/templates.tsx

-export const FieldSet: React.FC<FieldSetProps> = ({
-  children,
-  idSchema,
-  name,
-  required = false,
-  schema,
-  title,
-  uiSchema = {},
-}) => {
-  const [expanded, setExpanded] = React.useState(false);
-  const options = getUiOptions(uiSchema);
-  const { label: showLabel = true } = options;
-  const displayName = (options?.title as string) ?? schema?.title ?? title ?? name;
-  const onToggle = (e) => {
-    e.preventDefault();
-    setExpanded((current) => !current);
-  };
-  return showLabel && displayName ? (
-    <div id={`${idSchema.$id}_field-group`} className="co-dynamic-form__field-group">
-      <AccordionItem>
-        <AccordionToggle
-          id={`${idSchema.$id}_accordion-toggle`}
-          onClick={onToggle}
-          isExpanded={expanded}
-        >
-          <label
-            className={classnames({ 'co-required': required })}
-            htmlFor={`${idSchema.$id}_accordion-content`}
-          >
-            {_.startCase(displayName)}
-          </label>
-        </AccordionToggle>
-        <AccordionContent id={`${idSchema.$id}_accordion-content`} isHidden={!expanded}>
-          {children}
-        </AccordionContent>
-      </AccordionItem>
-    </div>
-  ) : (
-    <>{children}</>
-  );
-};
-


Moved to fields.tsx

TheRealJon · 2020-07-29T16:36:22Z

frontend/public/components/modals/configure-update-strategy-modal.tsx

+  const { showDescription = true } = props;
  return (
    <>
-      <div className="co-m-form-row">
-        <p>How should the pods be replaced when a new revision is created?</p>
-      </div>
-
+      {showDescription && (
+        <div className="co-m-form-row">
+          <p>{UPDATE_STRATEGY_DESCRIPTION}</p>
+        </div>
+      )}


Needed to override default description when one is provided through a descriptor or schema property.

spadgett

/lgtm

openshift-ci-robot · 2020-07-29T18:33:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: spadgett, TheRealJon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-07-30T01:00:20Z

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

openshift-ci-robot · 2020-07-30T01:00:23Z

@openshift-bot: This pull request references Bugzilla bug 1861433, which is invalid:

expected dependent Bugzilla bug 1854198 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-bot · 2020-07-31T01:00:23Z

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

openshift-ci-robot · 2020-07-31T01:00:27Z

@openshift-bot: This pull request references Bugzilla bug 1861433, which is invalid:

expected dependent Bugzilla bug 1854198 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

TheRealJon · 2020-07-31T14:04:47Z

/bugzilla refresh

openshift-ci-robot · 2020-07-31T14:04:51Z

@TheRealJon: This pull request references Bugzilla bug 1861433, which is invalid:

expected dependent Bugzilla bug 1854198 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-bot · 2020-08-01T01:00:49Z

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

openshift-ci-robot · 2020-08-01T01:00:56Z

@openshift-bot: This pull request references Bugzilla bug 1861433, which is invalid:

expected dependent Bugzilla bug 1854198 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-bot · 2020-08-02T01:00:46Z

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

openshift-ci-robot · 2020-08-02T01:00:49Z

@openshift-bot: This pull request references Bugzilla bug 1861433, which is invalid:

expected dependent Bugzilla bug 1854198 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-bot · 2020-08-03T01:34:48Z

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

openshift-ci-robot · 2020-08-03T01:34:51Z

@openshift-bot: This pull request references Bugzilla bug 1861433, which is invalid:

expected dependent Bugzilla bug 1854198 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

TheRealJon · 2020-08-03T13:47:01Z

/bugzilla refresh

openshift-ci-robot · 2020-08-03T13:47:08Z

@TheRealJon: This pull request references Bugzilla bug 1861433, which is valid. The bug has been moved to the POST state.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.z) matches configured target release for branch (4.5.z)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 1854198 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA))
dependent Bugzilla bug 1854198 targets the "4.6.0" release, which is one of the valid target releases: 4.6.0, 4.6.z
bug has dependents

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-08-20T04:37:54Z

@TheRealJon: All pull requests linked via external trackers have merged: openshift/console#6136. Bugzilla bug 1861433 has been moved to the MODIFIED state.

In response to this:

[release-4.5] Bug 1861433: Show descriptions for all dynamic form fields

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Bug 1861433: Show descriptions for all dynamic form fields

d442cef

Manual cherry pick of openshift#6054

openshift-ci-robot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Jul 28, 2020

openshift-ci-robot added bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. component/core Related to console core functionality labels Jul 28, 2020

openshift-ci-robot requested review from jeff-phillips-18 and spadgett July 28, 2020 15:41

openshift-ci-robot added the component/shared Related to console-shared label Jul 28, 2020

TheRealJon changed the title ~~Bug 1861433: Show descriptions for all dynamic form fields~~ [release-4.5] Bug 1861433: Show descriptions for all dynamic form fields Jul 28, 2020

TheRealJon commented Jul 29, 2020

View reviewed changes

spadgett approved these changes Jul 29, 2020

View reviewed changes

openshift-ci-robot assigned spadgett Jul 29, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2020

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Aug 3, 2020

jwforres added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Aug 20, 2020

openshift-merge-robot merged commit b8c0309 into openshift:release-4.5 Aug 20, 2020

spadgett added this to the v4.5.z milestone Aug 20, 2020

TheRealJon deleted the 1861433-bz branch January 27, 2022 21:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[release-4.5] Bug 1861433: Show descriptions for all dynamic form fields #6136

[release-4.5] Bug 1861433: Show descriptions for all dynamic form fields #6136

TheRealJon commented Jul 28, 2020

openshift-ci-robot commented Jul 28, 2020

TheRealJon commented Jul 28, 2020

openshift-ci-robot commented Jul 28, 2020

spadgett commented Jul 28, 2020

TheRealJon commented Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

TheRealJon Jul 29, 2020

spadgett left a comment

openshift-ci-robot commented Jul 29, 2020

openshift-bot commented Jul 30, 2020

openshift-ci-robot commented Jul 30, 2020

openshift-bot commented Jul 31, 2020

openshift-ci-robot commented Jul 31, 2020

TheRealJon commented Jul 31, 2020

openshift-ci-robot commented Jul 31, 2020

openshift-bot commented Aug 1, 2020

openshift-ci-robot commented Aug 1, 2020

openshift-bot commented Aug 2, 2020

openshift-ci-robot commented Aug 2, 2020

openshift-bot commented Aug 3, 2020

openshift-ci-robot commented Aug 3, 2020

TheRealJon commented Aug 3, 2020

openshift-ci-robot commented Aug 3, 2020

openshift-ci-robot commented Aug 20, 2020


		export const DescriptionField: React.FC<FieldProps> = ({ id, description }) =>
		const Description = ({ id, description }) =>

[release-4.5] Bug 1861433: Show descriptions for all dynamic form fields #6136

[release-4.5] Bug 1861433: Show descriptions for all dynamic form fields #6136

Conversation

TheRealJon commented Jul 28, 2020

openshift-ci-robot commented Jul 28, 2020

TheRealJon commented Jul 28, 2020

openshift-ci-robot commented Jul 28, 2020

spadgett commented Jul 28, 2020

TheRealJon commented Jul 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jul 29, 2020

openshift-bot commented Jul 30, 2020

openshift-ci-robot commented Jul 30, 2020

openshift-bot commented Jul 31, 2020

openshift-ci-robot commented Jul 31, 2020

TheRealJon commented Jul 31, 2020

openshift-ci-robot commented Jul 31, 2020

openshift-bot commented Aug 1, 2020

openshift-ci-robot commented Aug 1, 2020

openshift-bot commented Aug 2, 2020

openshift-ci-robot commented Aug 2, 2020

openshift-bot commented Aug 3, 2020

openshift-ci-robot commented Aug 3, 2020

TheRealJon commented Aug 3, 2020

openshift-ci-robot commented Aug 3, 2020

openshift-ci-robot commented Aug 20, 2020