From e299f4466b6c702d443729e37d6856d224377825 Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Mon, 13 May 2024 10:55:52 -0700 Subject: [PATCH] (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 --- .../components/Forms/MappedFieldsConfig.ts | 3 +- app/client/lib/koForm.css | 13 ++ app/client/lib/koForm.js | 7 +- app/client/models/RuleOwner.ts | 2 +- app/client/models/entities/ViewFieldRec.ts | 5 +- app/client/models/entities/ViewSectionRec.ts | 4 + app/client/ui/SortConfig.ts | 2 +- app/client/ui/VisibleFieldsConfig.ts | 2 +- app/client/widgets/ConditionalStyle.ts | 190 ++++++++++++------ test/nbrowser/gristUtils.ts | 4 + 10 files changed, 157 insertions(+), 75 deletions(-) diff --git a/app/client/components/Forms/MappedFieldsConfig.ts b/app/client/components/Forms/MappedFieldsConfig.ts index 651345c8..a1a2b32c 100644 --- a/app/client/components/Forms/MappedFieldsConfig.ts +++ b/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', ` diff --git a/app/client/lib/koForm.css b/app/client/lib/koForm.css index 92f84175..3cb042a5 100644 --- a/app/client/lib/koForm.css +++ b/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; +} diff --git a/app/client/lib/koForm.js b/app/client/lib/koForm.js index 37c28c4a..eb6ab060 100644 --- a/app/client/lib/koForm.js +++ b/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}); diff --git a/app/client/models/RuleOwner.ts b/app/client/models/RuleOwner.ts index 068c3e20..fbdd047d 100644 --- a/app/client/models/RuleOwner.ts +++ b/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; // 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; // List of columns ids that are used as rules for conditional styles. diff --git a/app/client/models/entities/ViewFieldRec.ts b/app/client/models/entities/ViewFieldRec.ts index fed7d80d..9861e3c0 100644 --- a/app/client/models/entities/ViewFieldRec.ts +++ b/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( diff --git a/app/client/models/entities/ViewSectionRec.ts b/app/client/models/entities/ViewSectionRec.ts index 729f79de..82eb2760 100644 --- a/app/client/models/entities/ViewSectionRec.ts +++ b/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({ diff --git a/app/client/ui/SortConfig.ts b/app/client/ui/SortConfig.ts index eff351fc..44c4f43c 100644 --- a/app/client/ui/SortConfig.ts +++ b/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 { diff --git a/app/client/ui/VisibleFieldsConfig.ts b/app/client/ui/VisibleFieldsConfig.ts index 56e4c61d..f4a36ec4 100644 --- a/app/client/ui/VisibleFieldsConfig.ts +++ b/app/client/ui/VisibleFieldsConfig.ts @@ -440,7 +440,7 @@ function unselectDeletedFields(selection: Set, event: {deleted: IField[] } export const cssDragRow = styled('div', ` - display: flex !important; + display: flex; align-items: center; margin: 0 16px 0px 0px; & > .kf_draggable_content { diff --git a/app/client/widgets/ConditionalStyle.ts b/app/client/widgets/ConditionalStyle.ts index f2ccdf23..0d201741 100644 --- a/app/client/widgets/ConditionalStyle.ts +++ b/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; // Helper field for refreshing current record data. private _dataChangeTrigger = Observable.create(this, 0); + // Rules columns with their respective rule index. + private _rulesColsWithIndex: Computed = 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(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; + } +`); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 8a9a0f1b..4c7fc626 100644 --- a/test/nbrowser/gristUtils.ts +++ b/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;