(core) Comments

Summary:
First iteration for comments system for Grist.
- Comments are stored in a generic metatable `_grist_Cells`
- Each comment is connected to a particular cell (hence the generic name of the table)
- Access level works naturally for records stored in this table
-- User can add/read comments for cells he can see
-- User can't update/remove comments that he doesn't own, but he can delete them by removing cells (rows/columns)
-- Anonymous users can't see comments at all.
- Each comment can have replies (but replies can't have more replies)

Comments are hidden by default, they can be enabled by COMMENTS=true env variable.
Some things for follow-up
- Avatars, currently the user's profile image is not shown or retrieved from the server
- Virtual rendering for comments list in creator panel. Currently, there is a limit of 200 comments.

Test Plan: New and existing tests

Reviewers: georgegevoian, paulfitz

Reviewed By: georgegevoian

Subscribers: paulfitz

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

app/client/components/BaseView.js

@@ -25,6 +25,8 @@ const {setTestState} = require('app/client/lib/testState');
 const {ExtraRows} = require('app/client/models/DataTableModelWithDiff');
 const {createFilterMenu} = require('app/client/ui/ColumnFilterMenu');
 const {closeRegisteredMenu} = require('app/client/ui2018/menus');
+const {COMMENTS} = require('app/client/models/features');
+
 
 /**
  * BaseView forms the basis for ViewSection classes.
@@ -85,6 +87,8 @@ function BaseView(gristDoc, viewSectionModel, options) {
     this._filteredRowSource.updateFilter(filterFunc);
   }));
 
+  this.rowSource = this._filteredRowSource;
+
   // Sorted collection of all rows to show in this view.
   this.sortedRows = rowset.SortedRowSet.create(this, null, this.tableModel.tableData);
 
@@ -238,7 +242,8 @@ BaseView.commonCommands = {
   showRawData: function() { this.showRawData().catch(reportError); },
 
   filterByThisCellValue: function() { this.filterByThisCellValue(); },
-  duplicateRows: function() { this._duplicateRows().catch(reportError); }
+  duplicateRows: function() { this._duplicateRows().catch(reportError); },
+  openDiscussion: function() { this.openDiscussionAtCursor(); },
 };
 
 /**
@@ -288,6 +293,30 @@ BaseView.prototype.activateEditorAtCursor = function(options) {
   builder.buildEditorDom(this.editRowModel, lazyRow, options || {});
 };
 
+
+/**
+ * Opens discussion panel at the cursor position. Returns true if discussion panel was opened.
+ */
+ BaseView.prototype.openDiscussionAtCursor = function(id) {
+  if (!COMMENTS().get()) { return false; }
+  var builder = this.activeFieldBuilder();
+  if (builder.isEditorActive()) {
+    return false;
+  }
+  var rowId = this.viewData.getRowId(this.cursor.rowIndex());
+  // LazyArrayModel row model which is also used to build the cell dom. Needed since
+  // it may be used as a key to retrieve the cell dom, which is useful for editor placement.
+  var lazyRow = this.getRenderedRowModel(rowId);
+  if (!lazyRow) {
+    // TODO scroll into view. For now, just don't start discussion.
+    return false;
+  }
+  this.editRowModel.assign(rowId);
+  builder.buildDiscussionPopup(this.editRowModel, lazyRow, id);
+  return true;
+};
+
+
 /**
  * Move the floating RowModel for editing to the current cursor position, and return it.
  *

app/client/components/GristDoc.ts

@@ -48,6 +48,7 @@ import {isNarrowScreen, mediaSmall, testId} from 'app/client/ui2018/cssVars';
 import {IconName} from 'app/client/ui2018/IconList';
 import {invokePrompt} from 'app/client/ui2018/modals';
 import {FieldEditor} from "app/client/widgets/FieldEditor";
+import {DiscussionPanel} from 'app/client/widgets/DiscussionEditor';
 import {MinimalActionGroup} from 'app/common/ActionGroup';
 import {ClientQuery} from "app/common/ActiveDocAPI";
 import {CommDocUsage, CommDocUserAction} from 'app/common/CommTypes';
@@ -66,6 +67,7 @@ import {
   bundleChanges,
   Computed,
   dom,
+  DomContents,
   Emitter,
   fromKo,
   Holder,
@@ -101,11 +103,11 @@ export interface TabOptions {
   category?: any;
 }
 
-const RightPanelTool = StringUnion("none", "docHistory", "validations");
+const RightPanelTool = StringUnion("none", "docHistory", "validations", "discussion");
 
 export interface IExtraTool {
   icon: IconName;
-  label: string;
+  label: DomContents;
   content: TabContent[]|IDomComponent;
 }
 
@@ -162,6 +164,7 @@ export class GristDoc extends DisposableWithEvents {
   private _lastOwnActionGroup: ActionGroupWithCursorPos|null = null;
   private _rightPanelTabs = new Map<string, TabContent[]>();
   private _docHistory: DocHistory;
+  private _discussionPanel: DiscussionPanel;
   private _rightPanelTool = createSessionObs(this, "rightPanelTool", "none", RightPanelTool.guard);
   private _viewLayout: ViewLayout|null = null;
   private _showGristTour = getUserOrgPrefObs(this.userOrgPrefs, 'showGristTour');
@@ -317,6 +320,7 @@ export class GristDoc extends DisposableWithEvents {
     this._actionLog = this.autoDispose(ActionLog.create({ gristDoc: this }));
     this._undoStack = this.autoDispose(UndoStack.create(openDocResponse.log, { gristDoc: this }));
     this._docHistory = DocHistory.create(this, this.docPageModel, this._actionLog);
+    this._discussionPanel = DiscussionPanel.create(this, this);
 
     // Tap into docData's sendActions method to save the cursor position with every action, so that
     // undo/redo can jump to the right place.
@@ -404,6 +408,7 @@ export class GristDoc extends DisposableWithEvents {
     return this.docPageModel.currentDocId.get()!;
   }
 
+  // DEPRECATED This is used only for validation, which is not used anymore.
   public addOptionsTab(label: string, iconElem: any, contentObj: TabContent[], options: TabOptions): IDisposable {
     this._rightPanelTabs.set(label, contentObj);
     // Return a do-nothing disposable, to satisfy the previous interface.
@@ -857,7 +862,7 @@ export class GristDoc extends DisposableWithEvents {
   public async recursiveMoveToCursorPos(
     cursorPos: CursorPos,
     setAsActiveSection: boolean,
-    silent: boolean = false): Promise<void> {
+    silent: boolean = false): Promise<boolean> {
     try {
       if (!cursorPos.sectionId) { throw new Error('sectionId required'); }
       if (!cursorPos.rowId) { throw new Error('rowId required'); }
@@ -931,11 +936,13 @@ export class GristDoc extends DisposableWithEvents {
       // even though the cursor is at right place, the scroll could not have yet happened
       // wait for a bit (scroll is done in a setTimeout 0)
       await delay(0);
+      return true;
     } catch (e) {
       console.debug(`_recursiveMoveToCursorPos(${JSON.stringify(cursorPos)}): ${e}`);
       if (!silent) {
         throw new UserError('There was a problem finding the desired cell.');
       }
+      return false;
     }
   }
 
@@ -1051,6 +1058,9 @@ export class GristDoc extends DisposableWithEvents {
         const content = this._rightPanelTabs.get("Validate Data");
         return content ? {icon: 'Validation', label: 'Validation Rules', content} : null;
       }
+      case 'discussion': {
+        return  {icon: 'Chat', label: this._discussionPanel.buildMenu(), content: this._discussionPanel};
+      }
       case 'none':
       default: {
         return null;

app/client/components/commandList.js

@@ -318,6 +318,10 @@ exports.groups = [{
       name: 'datepickerFocus',
       keys: ['Up', 'Down'],
       desc: null, // While editing a date cell, switch keyboard focus to the datepicker
+    }, {
+      name: 'openDiscussion',
+      keys: ['Mod+Alt+M'],
+      desc: 'Comment',
     }
   ],
 }, {

app/client/declarations.d.ts

@@ -56,6 +56,7 @@ declare module "app/client/components/BaseView" {
     public gristDoc: GristDoc;
     public cursor: Cursor;
     public sortedRows: SortedRowSet;
+    public rowSource: RowSource;
     public activeFieldBuilder: ko.Computed<FieldBuilder>;
     public selectedColumns: ko.Computed<ViewFieldRec[]>|null;
     public disableEditing: ko.Computed<boolean>;
@@ -69,6 +70,7 @@ declare module "app/client/components/BaseView" {
     public buildTitleControls(): DomArg;
     public getLoadingDonePromise(): Promise<void>;
     public activateEditorAtCursor(options?: Options): void;
+    public openDiscussionAtCursor(discussionId?: number): boolean;
     public onResize(): void;
     public prepareToPrint(onOff: boolean): void;
     public moveEditRowToCursor(): DataRowModel;
@@ -140,10 +142,19 @@ declare module "app/client/models/BaseRowModel" {
 
 declare module "app/client/models/MetaRowModel" {
   import BaseRowModel from "app/client/models/BaseRowModel";
+  import {ColValues} from 'app/common/DocActions';
+  import {SchemaTypes} from 'app/common/schema';
+
+  type NPartial<T> = {
+    [P in keyof T]?: T[P]|null;
+  };
+  type Values<T> = T extends keyof SchemaTypes ? NPartial<SchemaTypes[T]> : ColValues;
+
   namespace MetaRowModel {}
-  class MetaRowModel extends BaseRowModel {
+  class MetaRowModel<TName extends (keyof SchemaTypes)|undefined = undefined> extends BaseRowModel {
     public _isDeleted: ko.Observable<boolean>;
     public events: { trigger: (key: string) => void };
+    public updateColValues(colValues: Values<TName>): Promise<void>;
   }
   export = MetaRowModel;
 }

app/client/models/DocModel.ts

@@ -37,21 +37,21 @@ import {createValidationRec, ValidationRec} from 'app/client/models/entities/Val
 import {createViewFieldRec, ViewFieldRec} from 'app/client/models/entities/ViewFieldRec';
 import {createViewRec, ViewRec} from 'app/client/models/entities/ViewRec';
 import {createViewSectionRec, ViewSectionRec} from 'app/client/models/entities/ViewSectionRec';
+import {CellRec, createCellRec} from 'app/client/models/entities/CellRec';
 import {RefListValue} from 'app/common/gristTypes';
 import {decodeObject} from 'app/plugin/objtypes';
 
 // Re-export all the entity types available. The recommended usage is like this:
 //    import {ColumnRec, ViewFieldRec} from 'app/client/models/DocModel';
 export type {ColumnRec, DocInfoRec, FilterRec, PageRec, TabBarRec, TableRec, ValidationRec,
-    ViewFieldRec, ViewRec, ViewSectionRec};
-
+    ViewFieldRec, ViewRec, ViewSectionRec, CellRec};
 
 /**
  * Creates the type for a MetaRowModel containing a KoSaveableObservable for each field listed in
  * the auto-generated app/common/schema.ts. It represents the metadata record in the database.
  * Particular DocModel entities derive from this, and add other helpful computed values.
  */
-export type IRowModel<TName extends keyof SchemaTypes> = MetaRowModel & {
+export type IRowModel<TName extends keyof SchemaTypes> = MetaRowModel<TName> & {
   [ColId in keyof SchemaTypes[TName]]: KoSaveableObservable<SchemaTypes[TName][ColId]>;
 };
 
@@ -124,6 +124,7 @@ export class DocModel {
   public pages: MTM<PageRec> = this._metaTableModel("_grist_Pages", createPageRec);
   public rules: MTM<ACLRuleRec> = this._metaTableModel("_grist_ACLRules", createACLRuleRec);
   public filters: MTM<FilterRec> = this._metaTableModel("_grist_Filters", createFilterRec);
+  public cells: MTM<CellRec> = this._metaTableModel("_grist_Cells", createCellRec);
 
   public docInfoRow: DocInfoRec;
 

app/client/models/entities/CellRec.ts

@@ -0,0 +1,43 @@
+import {isCensored} from 'app/common/gristTypes';
+import * as ko from 'knockout';
+import {KoArray} from 'app/client/lib/koArray';
+import {jsonObservable} from 'app/client/models/modelUtil';
+import * as modelUtil from 'app/client/models/modelUtil';
+import {ColumnRec, DocModel, IRowModel, recordSet, refRecord, TableRec} from 'app/client/models/DocModel';
+
+
+export interface CellRec extends IRowModel<"_grist_Cells"> {
+  column: ko.Computed<ColumnRec>;
+  table: ko.Computed<TableRec>;
+  children: ko.Computed<KoArray<CellRec>>;
+  hidden: ko.Computed<boolean>;
+  parent: ko.Computed<CellRec>;
+
+  text: modelUtil.KoSaveableObservable<string|undefined>;
+  userName: modelUtil.KoSaveableObservable<string|undefined>;
+  timeCreated: modelUtil.KoSaveableObservable<number|undefined>;
+  timeUpdated: modelUtil.KoSaveableObservable<number|undefined>;
+  resolved: modelUtil.KoSaveableObservable<boolean|undefined>;
+  resolvedBy: modelUtil.KoSaveableObservable<string|undefined>;
+}
+
+export function createCellRec(this: CellRec, docModel: DocModel): void {
+  this.hidden = ko.pureComputed(() => isCensored(this.content()));
+  this.column = refRecord(docModel.columns, this.colRef);
+  this.table = refRecord(docModel.tables, this.tableRef);
+  this.parent = refRecord(docModel.cells, this.parentId);
+  this.children = recordSet(this, docModel.cells, 'parentId');
+  const properContent = modelUtil.savingComputed({
+    read: () => this.hidden() ? '{}' : this.content(),
+    write: (setter, val) => setter(this.content, val)
+  });
+  const optionJson = jsonObservable(properContent);
+
+  // Comments:
+  this.text = optionJson.prop('text');
+  this.userName = optionJson.prop('userName');
+  this.timeCreated = optionJson.prop('timeCreated');
+  this.timeUpdated = optionJson.prop('timeUpdated');
+  this.resolved = optionJson.prop('resolved');
+  this.resolvedBy = optionJson.prop('resolvedBy');
+}

app/client/models/entities/ColumnRec.ts

@@ -1,5 +1,6 @@
 import {KoArray} from 'app/client/lib/koArray';
-import {DocModel, IRowModel, recordSet, refRecord, TableRec, ViewFieldRec} from 'app/client/models/DocModel';
+import {CellRec, DocModel, IRowModel, recordSet,
+        refRecord, TableRec, ViewFieldRec} from 'app/client/models/DocModel';
 import {jsonObservable, ObjObservable} from 'app/client/models/modelUtil';
 import * as gristTypes from 'app/common/gristTypes';
 import {getReferencedTableId} from 'app/common/gristTypes';
@@ -55,7 +56,7 @@ export interface ColumnRec extends IRowModel<"_grist_Tables_column"> {
   visibleColModel: ko.Computed<ColumnRec>;
 
   disableModifyBase: ko.Computed<boolean>;    // True if column config can't be modified (name, type, etc.)
-  disableModify: ko.Computed<boolean>;        // True if column can't be modified or is being transformed.
+  disableModify: ko.Computed<boolean>;        // True if column can't be modified (is summary) or is being transformed.
   disableEditData: ko.Computed<boolean>;      // True to disable editing of the data in this column.
 
   isHiddenCol: ko.Computed<boolean>;
@@ -73,6 +74,7 @@ export interface ColumnRec extends IRowModel<"_grist_Tables_column"> {
   // (i.e. they aren't actually referenced but they exist in the visible column and are relevant to e.g. autocomplete)
   // `formatter` formats actual cell values, e.g. a whole list from the display column.
   formatter: ko.Computed<BaseFormatter>;
+  cells: ko.Computed<KoArray<CellRec>>;
 
   // Helper which adds/removes/updates column's displayCol to match the formula.
   saveDisplayFormula(formula: string): Promise<void>|undefined;
@@ -83,6 +85,7 @@ export function createColumnRec(this: ColumnRec, docModel: DocModel): void {
   this.widgetOptionsJson = jsonObservable(this.widgetOptions);
   this.viewFields = recordSet(this, docModel.viewFields, 'colRef');
   this.summarySource = refRecord(docModel.columns, this.summarySourceCol);
+  this.cells = recordSet(this, docModel.cells, 'colRef');
 
   // Is this an empty column (undecided if formula or data); denoted by an empty formula.
   this.isEmpty = ko.pureComputed(() => this.isFormula() && this.formula() === '');

app/client/models/features.ts

@@ -0,0 +1,12 @@
+import {getGristConfig} from 'app/common/urlUtils';
+import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals';
+import {localStorageBoolObs} from 'app/client/lib/localStorageObs';
+import {Observable} from 'grainjs';
+
+export function COMMENTS(): Observable<boolean> {
+  const G = getBrowserGlobals('document', 'window');
+  if (!G.window.COMMENTS) {
+    G.window.COMMENTS = localStorageBoolObs('feature-comments', Boolean(getGristConfig().featureComments));
+  }
+  return G.window.COMMENTS;
+}

app/client/models/rowset.ts

@@ -26,6 +26,7 @@ import {DisposableWithEvents} from 'app/common/DisposableWithEvents';
 import {CompareFunc, sortedIndex} from 'app/common/gutil';
 import {SkippableRows} from 'app/common/TableData';
 import {RowFilterFunc} from "app/common/RowFilterFunc";
+import {Observable} from 'grainjs';
 
 /**
  * Special constant value that can be used for the `rows` array for the 'rowNotify'
@@ -390,7 +391,10 @@ class RowGroupHelper<Value> extends RowSource {
 }
 
 // ----------------------------------------------------------------------
-
+/**
+ * Helper function that does map.get(key).push(r), creating an Array for the given key if
+ * necessary.
+ */
 function _addToMapOfArrays<K, V>(map: Map<K, V[]>, key: K, r: V): void {
   let arr = map.get(key);
   if (!arr) { map.set(key, arr = []); }
@@ -437,11 +441,6 @@ export class RowGrouping<Value> extends RowListener {
 
   // Implementation of the RowListener interface.
 
-  /**
-   * Helper function that does map.get(key).push(r), creating an Array for the given key if
-   * necessary.
-   */
-
   public onAddRows(rows: RowList) {
     const groupedRows = new Map();
     for (const r of rows) {
@@ -707,6 +706,41 @@ export class SortedRowSet extends RowListener {
   }
 }
 
+type RowTester = (rowId: RowId) => boolean;
+/**
+ * RowWatcher is a RowListener that maintains an observable function that checks whether a row
+ * is in the connected RowSource.
+ */
+export class RowWatcher extends RowListener {
+  /**
+   * Observable function that returns true if the row is in the connected RowSource.
+   */
+  public rowFilter: Observable<RowTester> = Observable.create(this, () => false);
+  // We count the number of times the row is added or removed from the source.
+  // In most cases row is added and removed only once.
+  private _rowCounter: Map<RowId, number> = new Map();
+
+  public clear() {
+    this._rowCounter.clear();
+    this.rowFilter.set(() => false);
+    this.stopListening();
+  }
+
+  protected onAddRows(rows: RowList) {
+    for (const r of rows) {
+      this._rowCounter.set(r, (this._rowCounter.get(r) || 0) + 1);
+    }
+    this.rowFilter.set((row) => (this._rowCounter.get(row) ?? 0) > 0);
+  }
+
+  protected onRemoveRows(rows: RowList) {
+    for (const r of rows) {
+      this._rowCounter.set(r, (this._rowCounter.get(r) || 0) - 1);
+    }
+    this.rowFilter.set((row) => (this._rowCounter.get(row) ?? 0) > 0);
+  }
+}
+
 function isSmallChange(rows: RowList) {
   return Array.isArray(rows) && rows.length <= 2;
 }

app/client/ui/AccountWidget.ts

@@ -147,7 +147,7 @@ const cssAccountWidget = styled('div', `
   white-space: nowrap;
 `);
 
-const cssUserIcon = styled('div', `
+export const cssUserIcon = styled('div', `
   height: 48px;
   width: 48px;
   padding: 8px;

app/client/ui/CellContextMenu.ts

@@ -2,6 +2,7 @@ import { allCommands } from 'app/client/components/commands';
 import { menuDivider, menuItemCmd } from 'app/client/ui2018/menus';
 import { IMultiColumnContextMenu } from 'app/client/ui/GridViewMenus';
 import { IRowContextMenu } from 'app/client/ui/RowContextMenu';
+import { COMMENTS } from 'app/client/models/features';
 import { dom } from 'grainjs';
 
 export function CellContextMenu(rowOptions: IRowContextMenu, colOptions: IMultiColumnContextMenu) {
@@ -9,6 +10,8 @@ export function CellContextMenu(rowOptions: IRowContextMenu, colOptions: IMultiC
   const { disableInsert, disableDelete, isViewSorted } = rowOptions;
   const { disableModify, isReadonly } = colOptions;
 
+  // disableModify is true if the column is a summary column or is being transformed.
+  // isReadonly is true for readonly mode.
   const disableForReadonlyColumn = dom.cls('disabled', Boolean(disableModify) || isReadonly);
   const disableForReadonlyView = dom.cls('disabled', isReadonly);
 
@@ -32,7 +35,7 @@ export function CellContextMenu(rowOptions: IRowContextMenu, colOptions: IMultiC
     colOptions.isFormula ?
       null :
       menuItemCmd(allCommands.clearValues, nameClearCells, disableForReadonlyColumn),
-    menuItemCmd(allCommands.clearColumns, nameClearColumns, disableForReadonlyColumn),
+      menuItemCmd(allCommands.clearColumns, nameClearColumns, disableForReadonlyColumn),
 
     ...(
       (numCols > 1 || numRows > 1) ? [] : [
@@ -40,6 +43,9 @@ export function CellContextMenu(rowOptions: IRowContextMenu, colOptions: IMultiC
         menuItemCmd(allCommands.copyLink, 'Copy anchor link'),
         menuDivider(),
         menuItemCmd(allCommands.filterByThisCellValue, `Filter by this value`),
+        menuItemCmd(allCommands.openDiscussion, 'Comment', dom.cls('disabled', (
+         isReadonly || numRows === 0 || numCols === 0
+        )), dom.hide(use => !use(COMMENTS())))
       ]
     ),
 
@@ -70,8 +76,7 @@ export function CellContextMenu(rowOptions: IRowContextMenu, colOptions: IMultiC
     menuDivider(),
 
     // deletes
-    menuItemCmd(allCommands.deleteRecords, nameDeleteRows,
-                dom.cls('disabled', disableDelete)),
+    menuItemCmd(allCommands.deleteRecords, nameDeleteRows, dom.cls('disabled', disableDelete)),
 
     menuItemCmd(allCommands.deleteFields, nameDeleteColumns, disableForReadonlyColumn),
 

app/client/ui/GridViewMenus.ts

@@ -37,7 +37,7 @@ export interface IMultiColumnContextMenu {
   // true for some columns, but not all.
   numColumns: number;
   numFrozen: number;
-  disableModify: boolean|'mixed';  // If the columns are read-only.
+  disableModify: boolean|'mixed';  // If the columns are read-only. Mixed for multiple columns where some are read-only.
   isReadonly: boolean;
   isRaw: boolean;
   isFiltered: boolean;            // If this view shows a proper subset of all rows in the table.

app/client/ui/TopBar.ts

@@ -12,6 +12,8 @@ import {docBreadcrumbs} from 'app/client/ui2018/breadcrumbs';
 import {basicButton} from 'app/client/ui2018/buttons';
 import {cssHideForNarrowScreen, testId, theme} from 'app/client/ui2018/cssVars';
 import {IconName} from 'app/client/ui2018/IconList';
+import {menuAnnotate} from 'app/client/ui2018/menus';
+import {COMMENTS} from 'app/client/models/features';
 import {waitGrainObs} from 'app/common/gutil';
 import * as roles from 'app/common/roles';
 import {Computed, dom, DomElementArg, makeTestId, MultiHolder, Observable, styled} from 'grainjs';
@@ -92,6 +94,14 @@ export function createTopBarDoc(owner: MultiHolder, appModel: AppModel, pageMode
 
     buildShareMenuButton(pageModel),
 
+    dom.maybe(use =>
+      (
+        use(pageModel.gristDoc)
+        && !use(use(pageModel.gristDoc)!.isReadonly)
+        && use(COMMENTS())
+      ),
+      () => buildShowDiscussionButton(pageModel)),
+
     dom.update(
       buildNotifyMenuButton(appModel.notifier, appModel),
       cssHideForNarrowScreen.cls(''),
@@ -101,6 +111,22 @@ export function createTopBarDoc(owner: MultiHolder, appModel: AppModel, pageMode
   ];
 }
 
+function buildShowDiscussionButton(pageModel: DocPageModel) {
+  return cssHoverCircle({ style: `margin: 5px; position: relative;` },
+    cssTopBarBtn('Chat', dom.cls('tour-share-icon')),
+    cssBeta('Beta'),
+    testId('open-discussion'),
+    dom.on('click', () => pageModel.gristDoc.get()!.showTool('discussion'))
+  );
+}
+
+const cssBeta = styled(menuAnnotate, `
+  position: absolute;
+  top: 4px;
+  right: -9px;
+  font-weight: bold;
+`);
+
 // Given the GristDoc instance, returns a rename function for the current active page.
 // If the current page is not able to be renamed or the new name is invalid, the function is a noop.
 function getRenamePageFn(gristDoc: GristDoc): (val: string) => Promise<void> {

app/client/ui2018/IconList.ts

@@ -39,6 +39,7 @@ export type IconName = "ChartArea" |
   "BarcodeQR" |
   "BarcodeQR2" |
   "CenterAlign" |
+  "Chat" |
   "Code" |
   "Collapse" |
   "Convert" |
@@ -75,6 +76,7 @@ export type IconName = "ChartArea" |
   "Lock" |
   "Log" |
   "Mail" |
+  "Message" |
   "Minus" |
   "MobileChat" |
   "MobileChat2" |
@@ -90,6 +92,7 @@ export type IconName = "ChartArea" |
   "Pivot" |
   "PivotLight" |
   "Plus" |
+  "Popup" |
   "Public" |
   "PublicColor" |
   "PublicFilled" |
@@ -165,6 +168,7 @@ export const IconList: IconName[] = ["ChartArea",
   "BarcodeQR",
   "BarcodeQR2",
   "CenterAlign",
+  "Chat",
   "Code",
   "Collapse",
   "Convert",
@@ -201,6 +205,7 @@ export const IconList: IconName[] = ["ChartArea",
   "Lock",
   "Log",
   "Mail",
+  "Message",
   "Minus",
   "MobileChat",
   "MobileChat2",
@@ -216,6 +221,7 @@ export const IconList: IconName[] = ["ChartArea",
   "Pivot",
   "PivotLight",
   "Plus",
+  "Popup",
   "Public",
   "PublicColor",
   "PublicFilled",

app/client/ui2018/cssVars.ts

@@ -50,6 +50,7 @@ export const colors = {
 
   light: new CustomProp('color-light', '#FFFFFF'),
   dark: new CustomProp('color-dark', '#262633'),
+  darkText: new CustomProp('color-dark-text', '#494949'),
   darkBg: new CustomProp('color-dark-bg', '#262633'),
   slate: new CustomProp('color-slate', '#929299'),
 

app/client/widgets/FieldBuilder.css

@@ -30,3 +30,18 @@
   border-radius: 5px;
   background-color: var(--grist-theme-right-panel-field-settings-button-bg, lightgrey);
 }
+
+.field-comment-indicator {
+  display: none;
+}
+
+.field-with-comments .field-comment-indicator {
+  display: block;
+  position: absolute;
+  top: 0;
+  right: 0;
+  width: 0;
+  height: 0;
+  border-top: 11px solid var(--grist-color-orange);
+  border-left: 11px solid transparent;
+}

app/client/widgets/FieldBuilder.ts

@@ -14,6 +14,7 @@ import { DataRowModel } from 'app/client/models/DataRowModel';
 import { ColumnRec, DocModel, ViewFieldRec } from 'app/client/models/DocModel';
 import { SaveableObjObservable, setSaveValue } from 'app/client/models/modelUtil';
 import { CombinedStyle, Style } from 'app/client/models/Styles';
+import { COMMENTS } from 'app/client/models/features';
 import { FieldSettingsMenu } from 'app/client/ui/FieldMenus';
 import { cssBlockedCursor, cssLabel, cssRow } from 'app/client/ui/RightPanelStyles';
 import { buttonSelect, cssButtonSelect } from 'app/client/ui2018/buttonSelect';
@@ -22,6 +23,7 @@ import { IOptionFull, menu, select } from 'app/client/ui2018/menus';
 import { DiffBox } from 'app/client/widgets/DiffBox';
 import { buildErrorDom } from 'app/client/widgets/ErrorDom';
 import { FieldEditor, saveWithoutEditor, setupEditorCleanup } from 'app/client/widgets/FieldEditor';
+import { CellDiscussionPopup, EmptyCell } from 'app/client/widgets/DiscussionEditor';
 import { openFormulaEditor } from 'app/client/widgets/FormulaEditor';
 import { NewAbstractWidget } from 'app/client/widgets/NewAbstractWidget';
 import { NewBaseEditor } from "app/client/widgets/NewBaseEditor";
@@ -37,6 +39,8 @@ import * as _ from 'underscore';
 
 const testId = makeTestId('test-fbuilder-');
 
+
+
 // Creates a FieldBuilder object for each field in viewFields
 export function createAllFieldWidgets(gristDoc: GristDoc, viewFields: ko.Computed<KoArray<ViewFieldRec>>,
                                       cursor: Cursor, options: { isPreview?: boolean } = {}) {
@@ -99,6 +103,7 @@ export class FieldBuilder extends Disposable {
   private readonly _widgetCons: ko.Computed<{create: (...args: any[]) => NewAbstractWidget}>;
   private readonly _docModel: DocModel;
   private readonly _readonly: Computed<boolean>;
+  private readonly _comments: ko.Computed<boolean>;
 
   public constructor(public readonly gristDoc: GristDoc, public readonly field: ViewFieldRec,
                      private _cursor: Cursor, private _options: { isPreview?: boolean } = {}) {
@@ -107,6 +112,7 @@ export class FieldBuilder extends Disposable {
     this._docModel = gristDoc.docModel;
     this.origColumn = field.column();
     this.options = field.widgetOptionsJson;
+    this._comments = ko.pureComputed(() => toKo(ko, COMMENTS())());
 
     this._readOnlyPureType = ko.pureComputed(() => this.field.column().pureType());
 
@@ -566,27 +572,47 @@ export class FieldBuilder extends Disposable {
     const errorInStyle = ko.pureComputed(() => Boolean(computedRule()?.error));
 
     const cellText = ko.pureComputed(() => this.field.textColor() || '');
-    const cllFill = ko.pureComputed(() => this.field.fillColor() || '');
+    const cellFill = ko.pureComputed(() => this.field.fillColor() || '');
 
+    const hasComment = koUtil.withKoUtils(ko.computed(() => {
+      if (this.isDisposed()) { return false; }   // Work around JS errors during field removal.
+      if (!this._comments()) { return false; }
+      if (this.gristDoc.isReadonlyKo()) { return false; }
+      const rowId = row.id();
+      const discussion = this.field.column().cells().all()
+        .find(d =>
+          d.rowId() === rowId
+          && !d.resolved()
+          && d.type() === gristTypes.CellInfoType.COMMENT
+          && !d.hidden()
+          && d.root());
+      return Boolean(discussion);
+    }).extend({ deferred: true })).onlyNotifyUnequal();
+
+    const domHolder = new MultiHolder();
+    domHolder.autoDispose(hasComment);
+    domHolder.autoDispose(widgetObs);
+    domHolder.autoDispose(computedFlags);
+    domHolder.autoDispose(errorInStyle);
+    domHolder.autoDispose(cellText);
+    domHolder.autoDispose(cellFill);
+    domHolder.autoDispose(computedRule);
+    domHolder.autoDispose(fontBold);
+    domHolder.autoDispose(fontItalic);
+    domHolder.autoDispose(fontUnderline);
+    domHolder.autoDispose(fontStrikethrough);
 
     return (elem: Element) => {
       this._rowMap.set(row, elem);
       dom(elem,
-          dom.autoDispose(widgetObs),
-          dom.autoDispose(computedFlags),
-          dom.autoDispose(errorInStyle),
-          dom.autoDispose(ruleText),
-          dom.autoDispose(computedRule),
-          dom.autoDispose(ruleFill),
-          dom.autoDispose(fontBold),
-          dom.autoDispose(fontItalic),
-          dom.autoDispose(fontUnderline),
-          dom.autoDispose(fontStrikethrough),
+          dom.autoDispose(domHolder),
           kd.style('--grist-cell-color', cellText),
-          kd.style('--grist-cell-background-color', cllFill),
+          kd.style('--grist-cell-background-color', cellFill),
           kd.style('--grist-rule-color', ruleText),
           kd.style('--grist-column-rule-background-color', ruleFill),
           this._options.isPreview ? null : kd.cssClass(this.field.formulaCssClass),
+          kd.toggleClass('field-with-comments', hasComment),
+          kd.maybe(hasComment, () => dom('div.field-comment-indicator')),
           kd.toggleClass("readonly", toKo(ko, this._readonly)),
           kd.maybe(isSelected, () => dom('div.selected_cursor',
                                          kd.toggleClass('active_cursor', isActive)
@@ -671,6 +697,43 @@ export class FieldBuilder extends Disposable {
     this.gristDoc.activeEditor.set(fieldEditor);
   }
 
+  public buildDiscussionPopup(editRow: DataRowModel, mainRowModel: DataRowModel, discussionId?: number) {
+    const owner = this.gristDoc.fieldEditorHolder;
+    const cellElem: Element = this._rowMap.get(mainRowModel)!;
+    if (this.columnTransform) {
+      this.columnTransform.finalize().catch(reportError);
+      return;
+    }
+    if (editRow._isAddRow.peek() || this._readonly.get()) {
+      return;
+    }
+
+    const cell = editRow.cells[this.field.colId()];
+    const value = cell && cell();
+    if (gristTypes.isCensored(value)) {
+      this._fieldEditorHolder.clear();
+      return;
+    }
+
+    const tableRef = this.field.viewSection.peek()!.tableRef.peek()!;
+
+    // Reuse fieldEditor holder to make sure only one popup/editor is attached to the cell.
+    const discussionHolder = MultiHolder.create(owner);
+    const discussions = EmptyCell.create(discussionHolder, {
+      gristDoc: this.gristDoc,
+      tableRef,
+      column: this.field.column.peek(),
+      rowId: editRow.id.peek(),
+    });
+    CellDiscussionPopup.create(discussionHolder, {
+      domEl: cellElem,
+      topic: discussions,
+      discussionId,
+      gristDoc: this.gristDoc,
+      closeClicked: () => owner.clear()
+    });
+  }
+
   public isEditorActive() {
     return !this._fieldEditorHolder.isEmpty();
   }

app/client/widgets/FieldEditor.ts

@@ -334,7 +334,7 @@ export class FieldEditor extends Disposable {
     let waitPromise: Promise<unknown>|null = null;
 
     if (isFormula) {
-      const formula = editor.getCellValue();
+      const formula = String(editor.getCellValue() ?? '');
       // Bundle multiple changes so that we can undo them in one step.
       if (isFormula !== col.isFormula.peek() || formula !== col.formula.peek()) {
         waitPromise = this._gristDoc.docData.bundleActions(null, () => Promise.all([

app/client/widgets/FormulaEditor.ts

@@ -298,10 +298,10 @@ export function openFormulaEditor(options: {
 
   // AsyncOnce ensures it's called once even if triggered multiple times.
   const saveEdit = asyncOnce(async () => {
-    const formula = editor.getCellValue();
+    const formula = String(editor.getCellValue());
     if (formula !== column.formula.peek()) {
       if (options.onSave) {
-        await options.onSave(column, formula as string);
+        await options.onSave(column, formula);
       } else {
         await column.updateColValues({formula});
       }