(core) Support reordering conditional styles

Summary: Conditional style rules can now be reordered by dragging and dropping them.

Test Plan: Browser test.

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D4251

app/client/components/Forms/MappedFieldsConfig.ts

@@ -198,11 +198,10 @@ const cssControlLabel = styled('div', `
 
 // TODO: reuse them
 const cssDragRow = styled('div', `
-  display: flex !important;
+  display: flex;
   align-items: center;
   margin: 0 16px 0px 0px;
   margin-bottom: 2px;
-  cursor: grab;
 `);
 
 const cssFieldEntry = styled('div', `

app/client/lib/koForm.css

@@ -136,6 +136,15 @@ div:hover > .kf_tooltip {
 
 .kf_draggable {
   display: inline-block;
+}
+
+.kf_draggable--vertical {
+  display: block;
+}
+
+/* Style the handle as grabbable, or the draggable element itself (if there is no handle). */
+.ui-sortable-handle,
+.kf_draggable:not(:has(.ui-sortable-handle)) {
   cursor: grab;
 }
 
@@ -724,3 +733,7 @@ fieldset:disabled {
   width: 100%;
   position: relative;
 }
+
+.kf_drag_container.ui-sortable {
+  overflow: auto;
+}

app/client/lib/koForm.js

@@ -436,6 +436,8 @@ exports.collapsible = function(contentFunc, isMountedCollapsed) {
  *                                        function on click.
  * @param {String}   options.axis         Determines if the list is displayed vertically 'y' or
  *                                        horizontally 'x'.
+ * @param {String}   options.handle       The handle of the draggable. Defaults to the element
+ *                                        itself.
  * @param {Boolean|Function} drag_indicator Include the drag indicator. Defaults to true. Accepts
  *                                          also a function that returns a dom element. In which
  *                                          case, it will be used to create the drag indicator.
@@ -473,13 +475,13 @@ exports.draggableList = function(contentArray, itemCreateFunc, options) {
           // Fix for JQueryUI bug where mousedown on draggable elements fail to blur
           // active element. See: https://bugs.jqueryui.com/ticket/4261
           dom.on('mousedown', () => G.document.activeElement.blur()),
+          kd.toggleClass('kf_draggable--vertical', options.axis === 'y'),
           kd.cssClass(options.itemClass),
           (options.drag_indicator ?
            (typeof options.drag_indicator === 'boolean' ?
             dom('span.kf_drag_indicator.glyphicon.glyphicon-option-vertical') :
             options.drag_indicator()
            ) : null),
-          kd.style('display', options.axis === 'x' ? 'inline-block' : 'block'),
           kd.domData('model', item),
           kd.maybe(removeFunc !== undefined && options.removeButton, function() {
             return dom('span.drag_delete.glyphicon.glyphicon-remove',
@@ -502,7 +504,8 @@ exports.draggableList = function(contentArray, itemCreateFunc, options) {
     axis: options.axis,
     tolerance: "pointer",
     forcePlaceholderSize: true,
-    placeholder: 'kf_draggable__placeholder--' + (options.axis === 'x' ? 'horizontal' : 'vertical')
+    placeholder: 'kf_draggable__placeholder--' + (options.axis === 'x' ? 'horizontal' : 'vertical'),
+    handle: options.handle,
   });
   if (reorderFunc === undefined) {
     G.$(list).sortable("option", {disabled: true});

app/client/models/RuleOwner.ts

@@ -11,7 +11,7 @@ export interface RuleOwner {
   // If this field (or column) has a list of conditional styling rules.
   hasRules: ko.Computed<boolean>;
   // List of rules.
-  rulesList: ko.Computed<[GristObjCode.List, ...number[]] | null>;
+  rulesList: modelUtil.KoSaveableObservable<[GristObjCode.List, ...number[]] | null>;
   // List of columns that are used as rules for conditional styles.
   rulesCols: ko.Computed<ColumnRec[]>;
   // List of columns ids that are used as rules for conditional styles.

app/client/models/entities/ViewFieldRec.ts

@@ -293,7 +293,10 @@ export function createViewFieldRec(this: ViewFieldRec, docModel: DocModel): void
   });
 
   this.tableId = ko.pureComputed(() => this.column().table().tableId());
-  this.rulesList = ko.pureComputed(() => this._fieldOrColumn().rules());
+  this.rulesList = modelUtil.savingComputed({
+    read: () => this._fieldOrColumn().rules(),
+    write: (setter, val) => setter(this._fieldOrColumn().rules, val)
+  });
   this.rulesCols = refListRecords(docModel.columns, ko.pureComputed(() => this._fieldOrColumn().rules()));
   this.rulesColsIds = ko.pureComputed(() => this.rulesCols().map(c => c.colId()));
   this.rulesStyles = modelUtil.fieldWithDefault(

app/client/models/entities/ViewSectionRec.ts

@@ -824,6 +824,10 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel):
 
   this.tableId = this.autoDispose(ko.pureComputed(() => this.table().tableId()));
   const rawSection = this.autoDispose(ko.pureComputed(() => this.table().rawViewSection()));
+  this.rulesList = modelUtil.savingComputed({
+    read: () => rawSection().rules(),
+    write: (setter, val) => setter(rawSection().rules, val)
+  });
   this.rulesCols = refListRecords(docModel.columns, ko.pureComputed(() => rawSection().rules()));
   this.rulesColsIds = ko.pureComputed(() => this.rulesCols().map(c => c.colId()));
   this.rulesStyles = modelUtil.savingComputed({

app/client/ui/SortConfig.ts

@@ -270,7 +270,7 @@ export class SortConfig extends Disposable {
 }
 
 const cssDragRow = styled('div', `
-  display: flex !important;
+  display: flex;
   align-items: center;
   margin: 0 16px 0px 0px;
   & > .kf_draggable_content {

app/client/ui/VisibleFieldsConfig.ts

@@ -440,7 +440,7 @@ function unselectDeletedFields(selection: Set<number>, event: {deleted: IField[]
 }
 
 export const cssDragRow = styled('div', `
-  display: flex !important;
+  display: flex;
   align-items: center;
   margin: 0 16px 0px 0px;
   & > .kf_draggable_content {

app/client/widgets/ConditionalStyle.ts

@@ -1,3 +1,4 @@
+import * as kf from 'app/client/lib/koForm';
 import {makeT} from 'app/client/lib/localization';
 import {GristDoc} from 'app/client/components/GristDoc';
 import {ColumnRec} from 'app/client/models/DocModel';
@@ -10,22 +11,31 @@ import {withInfoTooltip} from 'app/client/ui/tooltips';
 import {textButton} from 'app/client/ui2018/buttons';
 import {ColorOption, colorSelect} from 'app/client/ui2018/ColorSelect';
 import {theme, vars} from 'app/client/ui2018/cssVars';
+import {cssDragger} from 'app/client/ui2018/draggableList';
 import {icon} from 'app/client/ui2018/icons';
 import {setupEditorCleanup} from 'app/client/widgets/FieldEditor';
 import {cssError, openFormulaEditor} from 'app/client/widgets/FormulaEditor';
 import {isRaisedException, isValidRuleValue} from 'app/common/gristTypes';
-import {RowRecord} from 'app/plugin/GristData';
+import {GristObjCode, RowRecord} from 'app/plugin/GristData';
+import {decodeObject} from 'app/plugin/objtypes';
 import {Computed, Disposable, dom, DomContents, makeTestId, Observable, styled} from 'grainjs';
 import debounce = require('lodash/debounce');
 
 const testId = makeTestId('test-widget-style-');
 const t = makeT('ConditionalStyle');
 
+type ColumnRecAndIndex = [ColumnRec, number];
+
 export class ConditionalStyle extends Disposable {
   // Holds data from currently selected record (holds data only when this field has conditional styles).
   private _currentRecord: Computed<RowRecord | undefined>;
   // Helper field for refreshing current record data.
   private _dataChangeTrigger = Observable.create(this, 0);
+  // Rules columns with their respective rule index.
+  private _rulesColsWithIndex: Computed<ColumnRecAndIndex[]> = Computed.create(this, (use) => {
+    const rulesCols = use(this._ruleOwner.rulesCols);
+    return rulesCols.map((col, i) => [col, i]);
+  });
 
   constructor(
     private _label: string,
@@ -83,72 +93,17 @@ export class ConditionalStyle extends Disposable {
         dom.hide(use => use(this._ruleOwner.hasRules))
       ),
       dom.domComputedOwned(
-        use => use(this._ruleOwner.rulesCols),
+        use => use(this._rulesColsWithIndex),
         (owner, rules) =>
           cssRuleList(
             dom.show(use => rules.length > 0 && (!this._disabled || !use(this._disabled))),
-            ...rules.map((column, ruleIndex) => {
-              const textColor = this._buildStyleOption(owner, ruleIndex, 'textColor');
-              const fillColor = this._buildStyleOption(owner, ruleIndex, 'fillColor');
-              const fontBold = this._buildStyleOption(owner, ruleIndex, 'fontBold');
-              const fontItalic = this._buildStyleOption(owner, ruleIndex, 'fontItalic');
-              const fontUnderline = this._buildStyleOption(owner, ruleIndex, 'fontUnderline');
-              const fontStrikethrough = this._buildStyleOption(owner, ruleIndex, 'fontStrikethrough');
-              const save = async () => {
-                // This will save both options.
-                await this._ruleOwner.rulesStyles.save();
-              };
-              const currentValue = Computed.create(owner, use => {
-                const record = use(this._currentRecord);
-                if (!record) {
-                  return null;
-                }
-                const value = record[use(column.colId)];
-                return value ?? null;
-              });
-              const hasError = Computed.create(owner, use => {
-                return !isValidRuleValue(use(currentValue));
-              });
-              const errorMessage = Computed.create(owner, use => {
-                const value = use(currentValue);
-                return (!use(hasError) ? '' :
-                  isRaisedException(value) ? t('Error in style rule') :
-                    t('Rule must return True or False'));
-              });
-              return dom('div',
-                testId(`conditional-rule-${ruleIndex}`),
-                testId(`conditional-rule`), // for testing
-                cssLineLabel('IF...'),
-                cssColumnsRow(
-                  cssLeftColumn(
-                    this._buildRuleFormula(column.formula, column, hasError),
-                    cssRuleError(
-                      dom.text(errorMessage),
-                      dom.show(hasError),
-                      testId(`rule-error-${ruleIndex}`),
-                    ),
-                    colorSelect(
-                      {
-                        textColor: new ColorOption({color:textColor, allowsNone: true, noneText: 'default'}),
-                        fillColor: new ColorOption({color:fillColor, allowsNone: true, noneText: 'none'}),
-                        fontBold,
-                        fontItalic,
-                        fontUnderline,
-                        fontStrikethrough
-                      }, {
-                        onSave: save,
-                        placeholder: this._label || 'Conditional Style',
-                      }
-                    )
-                  ),
-                  cssRemoveButton(
-                    'Remove',
-                    testId(`remove-rule-${ruleIndex}`),
-                    dom.on('click', () => this._ruleOwner.removeRule(ruleIndex))
-                  )
-                )
-              );
-            })
+            kf.draggableList(rules, (rule: ColumnRecAndIndex) => this._buildRule(owner, rule), {
+              reorder: this._reorderRule.bind(this),
+              removeButton: false,
+              drag_indicator: cssDragger,
+              itemClass: cssDragRow.className,
+              handle: `.${cssDragger.className}`,
+            }),
           )
       ),
       cssRow(
@@ -162,6 +117,98 @@ export class ConditionalStyle extends Disposable {
     ];
   }
 
+  private _buildRule(owner: Disposable, rule: ColumnRecAndIndex) {
+    const [column, index] = rule;
+    const textColor = this._buildStyleOption(owner, index, 'textColor');
+    const fillColor = this._buildStyleOption(owner, index, 'fillColor');
+    const fontBold = this._buildStyleOption(owner, index, 'fontBold');
+    const fontItalic = this._buildStyleOption(owner, index, 'fontItalic');
+    const fontUnderline = this._buildStyleOption(owner, index, 'fontUnderline');
+    const fontStrikethrough = this._buildStyleOption(owner, index, 'fontStrikethrough');
+    const save = async () => {
+      // This will save both options.
+      await this._ruleOwner.rulesStyles.save();
+    };
+    const currentValue = Computed.create(owner, use => {
+      const record = use(this._currentRecord);
+      if (!record) {
+        return null;
+      }
+      const value = record[use(column.colId)];
+      return value ?? null;
+    });
+    const hasError = Computed.create(owner, use => {
+      return !isValidRuleValue(use(currentValue));
+    });
+    const errorMessage = Computed.create(owner, use => {
+      const value = use(currentValue);
+      return (!use(hasError) ? '' :
+        isRaisedException(value) ? t('Error in style rule') :
+          t('Rule must return True or False'));
+    });
+    return dom('div',
+      testId(`conditional-rule-${index}`),
+      testId(`conditional-rule`), // for testing
+      cssLineLabel(t('IF...')),
+      cssColumnsRow(
+        cssLeftColumn(
+          this._buildRuleFormula(column.formula, column, hasError),
+          cssRuleError(
+            dom.text(errorMessage),
+            dom.show(hasError),
+            testId(`rule-error-${index}`),
+          ),
+          colorSelect(
+            {
+              textColor: new ColorOption({color:textColor, allowsNone: true, noneText: 'default'}),
+              fillColor: new ColorOption({color:fillColor, allowsNone: true, noneText: 'none'}),
+              fontBold,
+              fontItalic,
+              fontUnderline,
+              fontStrikethrough
+            }, {
+              onSave: save,
+              placeholder: this._label || t('Conditional Style'),
+            }
+          )
+        ),
+        cssRemoveButton(
+          'Remove',
+          testId(`remove-rule-${index}`),
+          dom.on('click', () => this._ruleOwner.removeRule(index))
+        )
+      )
+    );
+  }
+
+  private async _reorderRule(rule: ColumnRecAndIndex, nextRule: ColumnRecAndIndex | null) {
+    const rulesList = decodeObject(this._ruleOwner.rulesList.peek());
+    if (!Array.isArray(rulesList) || rulesList.length === 0) {
+      throw new Error('No conditional style rules');
+    }
+
+    const ruleColRef = rule[0].id.peek();
+    const nextRuleColRef = nextRule?.[0].id.peek();
+    const rulesStyles = [...this._ruleOwner.rulesStyles.peek()];
+    const ruleColRefIndex = rulesList.indexOf(ruleColRef);
+
+    // Remove the rule.
+    rulesList.splice(ruleColRefIndex, 1);
+    const [ruleStyle] = rulesStyles.splice(ruleColRefIndex, 1);
+
+    // Insert the removed rule before the next rule.
+    const nextRuleColRefIndex = nextRuleColRef ? rulesList.indexOf(nextRuleColRef) : rulesList.length;
+    rulesList.splice(nextRuleColRefIndex, 0, ruleColRef);
+    rulesStyles.splice(nextRuleColRefIndex, 0, ruleStyle);
+
+    await this._gristDoc.docModel.docData.bundleActions("Reorder conditional rules", () =>
+      Promise.all([
+        this._ruleOwner.rulesList.setAndSave([GristObjCode.List, ...rulesList]),
+        this._ruleOwner.rulesStyles.setAndSave(rulesStyles),
+      ])
+    );
+  }
+
   private _buildStyleOption<T extends keyof Style>(owner: Disposable, index: number, option: T) {
     const obs = Computed.create(owner, use => {
       const styles = use(this._ruleOwner.rulesStyles);
@@ -213,7 +260,7 @@ const cssIcon = styled(icon, `
 
 const cssLabel = styled('div', `
   text-transform: uppercase;
-  margin: 16px 16px 12px 16px;
+  margin: 16px 16px 12px 0px;
   color: ${theme.text};
   font-size: ${vars.xsmallFontSize};
 `);
@@ -265,8 +312,7 @@ const cssRuleError = styled(cssError, `
 
 const cssColumnsRow = styled(cssRow, `
   align-items: flex-start;
-  margin-top: 0px;
-  margin-bottom: 0px;
+  margin: 0px 16px 0px 0px;
 `);
 
 const cssLeftColumn = styled('div', `
@@ -276,3 +322,13 @@ const cssLeftColumn = styled('div', `
   flex-direction: column;
   gap: 4px;
 `);
+
+const cssDragRow = styled('div', `
+  display: flex;
+  align-items: center;
+  & > .kf_draggable_content {
+    margin: 4px 0;
+    flex: 1 1 0px;
+    min-width: 0px;
+  }
+`);

test/nbrowser/gristUtils.ts

@@ -2349,6 +2349,10 @@ export function setFillColor(color: string) {
   return setColor(driver.find('.test-fill-input'), color);
 }
 
+export function getStyleRuleAt(nr: number) {
+  return driver.find(`.test-widget-style-conditional-rule-${nr}`);
+}
+
 export async function styleRulesCount() {
   const rules = await driver.findAll('.test-widget-style-conditional-rule');
   return rules.length;