Update cell enablement algorithm

Update cell `enabled` determination to the new enablement algorithm.

Note that the cell `enabled` still prefers `ownProps` (except for the
form wide readonly flag) over any other determination for performance
gains. The Table (or other renderers dispatching to cells) usually
already determined whether the cell shall be enabled and therefore hand
the correct `enabled` prop over. If that is not wanted, the new
enablement algorithm will kick in when not setting this prop.

Also adds more `isInherentlyEnabled` test cases and fixes the JSON
Schema `readOnly` lookup.

Author: sdirix

packages/core/src/util/cell.ts

@@ -25,26 +25,20 @@
 import isEmpty from 'lodash/isEmpty';
 import union from 'lodash/union';
 import { getConfig, getData, getErrorAt, getSchema, getAjv } from '../reducers';
-import {
-  AnyAction,
-  Dispatch,
-  formatErrorMessage,
-  isEnabled,
-  isVisible,
-  OwnPropsOfControl,
-  OwnPropsOfEnum,
-  Resolve,
-  StatePropsOfScopedRenderer
-} from '.';
 import {
   DispatchPropsOfControl,
   EnumOption,
   enumToEnumOptionMapper,
-  mapDispatchToControlProps
+  mapDispatchToControlProps,
+  OwnPropsOfControl,
+  OwnPropsOfEnum,
+  StatePropsOfScopedRenderer
 } from './renderer';
 import { JsonFormsState } from '../store';
 import { JsonFormsCellRendererRegistryEntry } from '../reducers/cells';
-import { JsonSchema } from '..';
+import { JsonSchema } from '../models/jsonSchema';
+import { isInherentlyEnabled, isVisible } from './runtime';
+import { AnyAction, Dispatch, formatErrorMessage, Resolve } from '.';
 
 export { JsonFormsCellRendererRegistryEntry };
 
@@ -109,17 +103,38 @@ export const mapStateToCellProps = (
     ownProps.visible !== undefined
       ? ownProps.visible
       : isVisible(uischema, rootData, undefined, getAjv(state));
-  const readonly = state.jsonforms.readonly;
-  const enabled =
-    !readonly &&
-    (ownProps.enabled !== undefined
-      ? ownProps.enabled
-      : isEnabled(uischema, rootData, undefined, getAjv(state)));
+
+  const rootSchema = getSchema(state);
+  const config = getConfig(state);
+
+  /* When determining the enabled state of cells we take a shortcut: At the
+   * moment it's only possible to configure enablement and disablement at the
+   * control level. Therefore the renderer using the cell, for example a 
+   * table renderer, determines whether a cell is enabled and should hand
+   * over the prop themselves. If that prop was given, we prefer it over
+   * anything else to save evaluation effort (except for the global readonly
+   * flag). For example it would be quite expensive to evaluate the same ui schema
+   * rule again and again for each cell of a table. */
+  let enabled;
+  if (state.jsonforms.readonly === true) {
+    enabled = false;
+  } else if (typeof ownProps.enabled === 'boolean') {
+    enabled = ownProps.enabled;
+  } else {
+    enabled = isInherentlyEnabled(
+      state,
+      ownProps,
+      uischema,
+      schema || rootSchema,
+      rootData,
+      config
+    );
+  }
+
   const errors = formatErrorMessage(
     union(getErrorAt(path, schema)(state).map(error => error.message))
   );
   const isValid = isEmpty(errors);
-  const rootSchema = getSchema(state);
 
   return {
     data: Resolve.data(rootData, path),

packages/core/src/util/renderer.ts

@@ -785,7 +785,7 @@ export const mapStateToLayoutProps = (
     state,
     ownProps,
     uischema,
-    ownProps.schema,
+    undefined, // layouts have no associated schema
     rootData,
     config
   );

packages/core/src/util/runtime.ts

@@ -181,11 +181,16 @@ export const isEnabled = (
   return true;
 };
 
+/**
+ * Indicates whether the given `uischema` element shall be enabled or disabled.
+ * Checks the global readonly flag, uischema rule, uischema options (including the config),
+ * the schema and the enablement indicator of the parent.
+ */
 export const isInherentlyEnabled = (
   state: JsonFormsState,
   ownProps: any,
   uischema: UISchemaElement,
-  schema: JsonSchema & { readOnly?: boolean },
+  schema: JsonSchema & { readOnly?: boolean } | undefined,
   rootData: any,
   config: any
 ) => {
@@ -207,8 +212,8 @@ export const isInherentlyEnabled = (
   if (typeof config?.readOnly === 'boolean') {
     return !config.readOnly;
   }
-  if (typeof schema?.readOnly === 'boolean') {
-    return !schema.readOnly;
+  if (schema?.readOnly === true) {
+    return false;
   }
   if (typeof ownProps?.enabled === 'boolean') {
     return ownProps.enabled;

packages/core/test/util/runtime.test.ts

@@ -592,7 +592,7 @@ test('isInherentlyEnabled disabled by uischema over ownProps', t => {
   t.false(
     isInherentlyEnabled(
       null,
-      { enabled: false },
+      { enabled: true },
       ({ options: { readonly: true } } as unknown) as ControlElement,
       null,
       null,
@@ -601,6 +601,32 @@ test('isInherentlyEnabled disabled by uischema over ownProps', t => {
   );
 });
 
+test('isInherentlyEnabled enabled by uischema over schema', t => {
+  t.true(
+    isInherentlyEnabled(
+      null,
+      null,
+      ({ options: { readonly: false } } as unknown) as ControlElement,
+      { readOnly: true },
+      null,
+      null
+    )
+  );
+});
+
+test('isInherentlyEnabled disabled by ownProps over schema enablement', t => {
+  t.false(
+    isInherentlyEnabled(
+      null,
+      { enabled: false},
+      null,
+      { readOnly: false },
+      null,
+      null
+    )
+  );
+});
+
 test('isInherentlyEnabled disabled by uischema over schema', t => {
   t.false(
     isInherentlyEnabled(
@@ -712,6 +738,42 @@ test('isInherentlyEnabled enabled by config over ownProps', t => {
   );
 });
 
+test('isInherentlyEnabled enabled by uischema over config', t => {
+  t.true(
+    isInherentlyEnabled(
+      null,
+      null,
+      ({ options: { readonly: false } } as unknown) as ControlElement,
+      null,
+      null,
+      { readonly: true }
+    )
+  );
+});
+
+test('isInherentlyEnabled prefer readonly over readOnly', t => {
+  t.true(
+    isInherentlyEnabled(
+      null,
+      null,
+      ({ options: { readonly: false, readOnly: true } } as unknown) as ControlElement,
+      null,
+      null,
+      null
+    )
+  );
+  t.false(
+    isInherentlyEnabled(
+      null,
+      null,
+      ({ options: { readonly: true, readOnly: false } } as unknown) as ControlElement,
+      null,
+      null,
+      null
+    )
+  );
+});
+
 test('isInherentlyEnabled enabled', t => {
   t.true(isInherentlyEnabled(null, null, null, null, null, null));
 });