(core) Making cells colors effective also in Card and Card List views

Summary:
 . Makes cell color work well in comparison mode
 . Do not apply cell color to the add new row
 . Allow to change color for all widget (including changing color for the checkbox and the switch widget)
 . Fix an issue that was setting color to black when opening the picker
 . Do not apply color to invalid cell

Test Plan: . Added nbrowser/CellColor

Reviewers: paulfitz, dsagal

Reviewed By: dsagal

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

app/client/components/GridView.js

@@ -935,10 +935,6 @@ GridView.prototype.buildDom = function() {
             //TODO: Ensure that fields in a row resize when
             //a cell in that row becomes larger
             kd.style('borderRightWidth', v.borderWidthPx),
-            kd.style('color', field.textColor),
-            // If making a comparison, use the background exclusively for
-            // marking that up.
-            self.comparison ? null : kd.style('background-color', () => (row._isAddRow() || isSelected()) ? '' : field.fillColor()),
 
             kd.toggleClass('selected', isSelected),
             fieldBuilder.buildDomWithCursor(row, isCellActive, isCellSelected)

app/client/components/viewCommon.css

@@ -55,10 +55,14 @@
   text-overflow: ellipsis;
   width: 100%;
   height: 100%;
+  background-color: var(--grist-diff-background-color, var(--grist-cell-background-color, unset));
+  --grist-actual-cell-color: var(--grist-diff-color, var(--grist-cell-color));
+  color: var(--grist-actual-cell-color, unset);
 }
 
 .field_clip.invalid {
   background-color: #ffb6c1;
+  color: unset;
 }
 
 .field_clip.invalid:empty {
@@ -159,20 +163,28 @@
 
 .diff-local, .diff-local-add {
   background-color: #dfdfff;
+  --grist-diff-background-color: #dfdfff;
+  --grist-diff-color: black;
 }
 
 .diff-parent, .diff-remote-remove {
   background-color: #ffdfdf;
+  --grist-diff-background-color: #ffdfdf;
+  --grist-diff-color: black;
   text-decoration: line-through;
 }
 
 .diff-local-remove {
   background-color: #dfdfdf;
+  --grist-diff-background-color: #dfdfdf;
+  --grist-diff-color: black;
   text-decoration: line-through;
 }
 
 .diff-remote, .diff-remote-add {
   background-color: #afffaf;
+  --grist-diff-background-color: #afffaf;
+  --grist-diff-color: black;
 }
 
 .diff-common {

app/client/lib/koDom.js

@@ -139,7 +139,14 @@ exports.attr = attr;
  */
 function style(property, valueOrFunc) {
   return makeBinding(valueOrFunc, function(elem, value) {
-    elem.style[property] = value;
+    // `style.setProperty` must be use to set custom property (ie: properties starting with '--').
+    // However since it does not support camelCase property, we still need to use the other form
+    // `elem.style[prop] = val;` for other properties.
+    if (property.startsWith('--')) {
+      elem.style.setProperty(property, value);
+    } else {
+      elem.style[property] = value;
+    }
   });
 }
 exports.style = style;

app/client/models/entities/ViewFieldRec.ts

@@ -202,13 +202,14 @@ export function createViewFieldRec(this: ViewFieldRec, docModel: DocModel): void
   this.disableEditData = ko.pureComputed(() => this.column().disableEditData());
 
   this.textColor = modelUtil.fieldWithDefault(
-    this.widgetOptionsJson.prop('textColor') as modelUtil.KoSaveableObservable<string>, "#000000");
+    this.widgetOptionsJson.prop('textColor') as modelUtil.KoSaveableObservable<string>, '');
 
-  const fillColorProp = this.widgetOptionsJson.prop('fillColor') as modelUtil.KoSaveableObservable<string>;
+  const fillColorProp = modelUtil.fieldWithDefault(
+    this.widgetOptionsJson.prop('fillColor') as modelUtil.KoSaveableObservable<string>, "#FFFFFF00");
   // Store empty string in place of the default white color, so that we can keep it transparent in
   // GridView, to avoid interfering with zebra stripes.
   this.fillColor = modelUtil.savingComputed({
     read: () => fillColorProp(),
-    write: (setter, val) => setter(fillColorProp, val === '#ffffff' ? '' : val),
+    write: (setter, val) => setter(fillColorProp, val.toUpperCase() === '#FFFFFF' ? '' : val),
   });
 }

app/client/ui2018/buttonSelect.ts

@@ -71,7 +71,7 @@ export function alignmentSelect(obs: Observable<string>) {
 }
 
 /**
- * Color selector button. Observable should contain a hex color value, e.g. #a4ba23
+ * Color selector button. Observable should contain a hex color value, e.g. #a4ba23.
  */
 export function colorSelect(value: Observable<string>, save: (val: string) => Promise<void>,
                             ...domArgs: DomElementArg[]) {
@@ -89,12 +89,12 @@ export function colorSelect(value: Observable<string>, save: (val: string) => Pr
     // by the server.
     cssColorPicker(
       {type: 'color'},
-      dom.attr('value', value),
+      dom.attr('value', (use) => use(value).slice(0, 7)),
       dom.on('input', setValue),
       dom.on('change', onSave)
     ),
-    dom.style('background-color', (use) => use(value) || '#ffffff'),
+    dom.style('background-color', (use) => use(value) || '#000000'),
-    cssColorBtn.cls('-dark', (use) => isColorDark(use(value) || '#ffffff')),
+    cssColorBtn.cls('-dark', (use) => isColorDark(use(value) || '#000000')),
     cssColorIcon('Dots'),
     ...domArgs
   );

app/client/widgets/CheckBox.css

@@ -42,8 +42,8 @@
   position: relative;
   width: 3px;
   height: 12px;
-  background-color: #606060;
+  background-color: var(--grist-cell-color, #606060);
-  border: 1px solid #606060;
+  border: 1px solid var(--grist-cell-color, #606060);
   left: 3px;
   top: -5px;
 }
@@ -52,7 +52,7 @@
   position: relative;
   width: 3px;
   height: 3px;
-  background-color: #606060;
+  background-color: var(--grist-cell-color, #606060);
-  border: 1px solid #606060;
+  border: 1px solid var(--grist-cell-color, #606060);
   top: 7px;
 }

app/client/widgets/DiffBox.ts

@@ -2,6 +2,7 @@ import { DataRowModel } from 'app/client/models/DataRowModel';
 import { NewAbstractWidget } from 'app/client/widgets/NewAbstractWidget';
 import { CellValue } from 'app/common/DocActions';
 import { isVersions } from 'app/common/gristTypes';
+import { inlineStyle } from 'app/common/gutil';
 import { BaseFormatter } from 'app/common/ValueFormatter';
 import { Diff, DIFF_DELETE, DIFF_INSERT, diff_match_patch as DiffMatchPatch, DIFF_EQUAL } from 'diff-match-patch';
 import { Computed, dom } from 'grainjs';
@@ -37,6 +38,8 @@ export class DiffBox extends NewAbstractWidget {
       dom.autoDispose(formattedValue),
       dom.style('text-align', this.options.prop('alignment')),
       dom.cls('text_wrapping', (use) => Boolean(use(this.options.prop('wrap')))),
+      inlineStyle('--grist-diff-color', '#000000'),
+      inlineStyle('--grist-diff-background-color', '#00000000'),
       dom.forEach(formattedValue, ([code, txt]) => {
         if (code === DIFF_DELETE) {
           return dom("span.diff-parent", txt);

app/client/widgets/FieldBuilder.ts

@@ -128,11 +128,13 @@ export class FieldBuilder extends Disposable {
       write(widget) {
         // Reset the entire JSON, so that all options revert to their defaults.
 
+        const previous = this.options.peek();
         this.options.setAndSave({
           widget,
-          // persists color settings across widgets
+          // Persists color settings across widgets (note: we cannot use `field.fillColor` to get the
-          fillColor: field.fillColor.peek(),
+          // current value because it returns a default value for `undefined`. Same for `field.textColor`.
-          textColor: field.textColor.peek()
+          fillColor: previous.fillColor,
+          textColor: previous.textColor,
         }).catch(reportError);
       }
     });
@@ -418,7 +420,9 @@ export class FieldBuilder extends Disposable {
           kd.scope(widgetObs, (widget: NewAbstractWidget) => {
             if (this.isDisposed()) { return null; }   // Work around JS errors during field removal.
             const cellDom = widget ? widget.buildDom(row) : buildErrorDom(row, this.field);
-            return dom(cellDom, kd.toggleClass('has_cursor', isActive));
+            return dom(cellDom, kd.toggleClass('has_cursor', isActive),
+                       kd.style('--grist-cell-color', this.field.textColor),
+                       kd.style('--grist-cell-background-color', this.field.fillColor));
           })
          );
     };

app/client/widgets/HyperLinkTextBox.ts

@@ -28,7 +28,7 @@ export class HyperLinkTextBox extends NTextBox {
           // from running on the same process as Grist:
           // https://developers.google.com/web/tools/lighthouse/audits/noopener
           dom.attr('rel', 'noopener noreferrer'),
-          cssLinkIcon('FieldLink'),
+          cssLinkIcon('FieldLink', testId('tb-link-icon')),
           testId('tb-link')
         )
       ),
@@ -62,10 +62,10 @@ function _constructUrl(value: string): string {
 }
 
 const cssFieldClip = styled('div.field_clip', `
-  color: ${colors.lightGreen};
+  color: var(--grist-actual-cell-color, ${colors.lightGreen});
 `);
 
 const cssLinkIcon = styled(icon, `
-  background-color: ${colors.lightGreen};
+  background-color: var(--grist-actual-cell-color, ${colors.lightGreen});
   margin: -1px 2px 2px 0;
 `);

app/client/widgets/NewAbstractWidget.ts

@@ -28,10 +28,14 @@ export abstract class NewAbstractWidget extends Disposable {
 
   protected options: SaveableObjObservable<any>;
   protected valueFormatter: Observable<BaseFormatter>;
+  protected textColor: Observable<string>;
+  protected fillColor: Observable<string>;
 
   constructor(protected field: ViewFieldRec) {
     super();
     this.options = field.widgetOptionsJson;
+    this.textColor = fromKo(this.field.textColor);
+    this.fillColor = fromKo(this.field.fillColor);
 
     // Note that its easier to create a knockout computed from the several knockout observables,
     // but then we turn it into a grainjs observable.
@@ -57,7 +61,7 @@ export abstract class NewAbstractWidget extends Disposable {
       cssRow(
         cssHalfWidth(
           colorSelect(
-            fromKo(this.field.textColor) as Observable<string>,
+            this.textColor,
             (val) => this.field.textColor.saveOnly(val),
             testId('text-color')
           ),
@@ -65,7 +69,7 @@ export abstract class NewAbstractWidget extends Disposable {
         ),
         cssHalfWidth(
           colorSelect(
-            fromKo(this.field.fillColor) as Observable<string>,
+            this.fillColor,
             (val) => this.field.fillColor.saveOnly(val),
             testId('fill-color')
           ),

app/client/widgets/Switch.css

@@ -33,7 +33,7 @@
 }
 
 .switch_on > .switch_slider {
-  background-color: #2CB0AF;
+  background-color: var(--grist-cell-color, #2CB0AF);
 }
 
 .switch_on > .switch_circle {

app/common/gutil.ts

@@ -1,5 +1,5 @@
 import {delay} from 'app/common/delay';
-import {Listener, Observable} from 'grainjs';
+import {BindableValue, DomElementMethod, Listener, Observable, subscribeElem} from 'grainjs';
 import {Observable as KoObservable} from 'knockout';
 import constant = require('lodash/constant');
 import identity = require('lodash/identity');
@@ -727,6 +727,17 @@ export async function waitGrainObs<T>(observable: Observable<T>,
   return res;
 }
 
+
+// `dom.style` does not work here because custom css property (ie: `--foo`) needs to be set using
+// `style.setProperty` (credit: https://vanseodesign.com/css/custom-properties-and-javascript/).
+// TODO: consider making PR to fix `dom.style` in grainjs.
+export function inlineStyle(property: string, valueObs: BindableValue<string>): DomElementMethod {
+  return (elem) => subscribeElem(elem, valueObs, (val) => {
+    elem.style.setProperty(property, val);
+  });
+}
+
+
 /**
  * Class to maintain a chain of promise-returning callbacks. All scheduled callbacks will be
  * called in order as long as the previous one is successful. If a callback fails is rejected,