(core) Editing summary columns widget options

Summary:
- Allowing changing description and all widget options (except choices) for summary columns
- Making text error in toast notification selectable

Test Plan: Added new tests

Reviewers: georgegevoian

Reviewed By: georgegevoian

Subscribers: georgegevoian

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

app/client/models/NotifyModel.ts

@@ -381,7 +381,7 @@ export class Notifier extends Disposable implements INotifier {
         return dom('div',
           dom.forEach(appErrors, (appErr: IAppError) =>
             (where === 'toast' && appErr.seen ? null :
-              dom('div', timeFormat('T', new Date(appErr.timestamp)), ' ',
+              dom('div', {tabIndex: "-1"}, timeFormat('T', new Date(appErr.timestamp)), ' ',
                   appErr.error.message, testId('notification-app-error'))
             )
           ),

sandbox/grist/test_summary.py

@@ -4,6 +4,7 @@ files: test_summary.py and test_summary2.py.
 """
 
 import logging
+import unittest
 
 import actions
 import summary
@@ -960,49 +961,49 @@ class Address:
     old = '{"widget":"Spinner","alignment":"center"}'
     self.assertTrue(widgetOptions(new, old))
 
-    # Can update but must leave other options.
+    # Can update but must leave choices options.
-    new = '{"widget":"TextBox","cant":"center"}'
+    new = '{"widget":"TextBox","choices":"center"}'
-    old = '{"widget":"Spinner","cant":"center"}'
+    old = '{"widget":"Spinner","choices":"center"}'
     self.assertTrue(widgetOptions(new, old))
 
     # Can't add protected property when old was empty.
-    new = '{"widget":"TextBox","cant":"new"}'
+    new = '{"widget":"TextBox","choices":"new"}'
     old = None
     self.assertFalse(widgetOptions(new, old))
 
     # Can't remove when there was a protected property.
     new = None
-    old = '{"widget":"TextBox","cant":"old"}'
+    old = '{"widget":"TextBox","choices":"old"}'
     self.assertFalse(widgetOptions(new, old))
 
     # Can't update by omitting.
     new = '{"widget":"TextBox"}'
-    old = '{"widget":"TextBox","cant":"old"}'
+    old = '{"widget":"TextBox","choices":"old"}'
     self.assertFalse(widgetOptions(new, old))
 
     # Can't update by changing.
-    new = '{"widget":"TextBox","cant":"new"}'
+    new = '{"widget":"TextBox","choices":"new"}'
-    old = '{"widget":"TextBox","cant":"old"}'
+    old = '{"widget":"TextBox","choices":"old"}'
     self.assertFalse(widgetOptions(new, old))
 
     # Can't update by adding.
-    new = '{"widget":"TextBox","cant":"new"}'
+    new = '{"widget":"TextBox","choices":"new"}'
     old = '{"widget":"TextBox"}'
     self.assertFalse(widgetOptions(new, old))
 
     # Can update objects
-    new = '{"widget":"TextBox","alignment":{"prop":1},"cant":{"prop":1}}'
+    new = '{"widget":"TextBox","alignment":{"prop":1},"choices":{"prop":1}}'
-    old = '{"widget":"TextBox","alignment":{"prop":2},"cant":{"prop":1}}'
+    old = '{"widget":"TextBox","alignment":{"prop":2},"choices":{"prop":1}}'
     self.assertTrue(widgetOptions(new, old))
 
     # Can't update objects
-    new = '{"widget":"TextBox","cant":{"prop":1}}'
+    new = '{"widget":"TextBox","choices":{"prop":1}}'
-    old = '{"widget":"TextBox","cant":{"prop":2}}'
+    old = '{"widget":"TextBox","choices":{"prop":2}}'
     self.assertFalse(widgetOptions(new, old))
 
     # Can't update lists
-    new = '{"widget":"TextBox","cant":[1, 2]}'
+    new = '{"widget":"TextBox","choices":[1, 2]}'
-    old = '{"widget":"TextBox","cant":[2, 1]}'
+    old = '{"widget":"TextBox","choices":[2, 1]}'
     self.assertFalse(widgetOptions(new, old))
 
     # Can update lists
@@ -1011,7 +1012,10 @@ class Address:
     self.assertTrue(widgetOptions(new, old))
 
     # Can update without changing list.
-    new = '{"widget":"TextBox","dontChange":[1, 2]}'
+    new = '{"widget":"TextBox","choices":[1, 2]}'
-    old = '{"widget":"Spinner","dontChange":[1, 2]}'
+    old = '{"widget":"Spinner","choices":[1, 2]}'
     self.assertTrue(widgetOptions(new, old))
   # pylint: enable=R0915
+
+if __name__ == "__main__":
+  unittest.main()

sandbox/grist/useractions.py

@@ -192,8 +192,8 @@ def allowed_summary_change(key, updated, original):
   """
   Checks if summary group by column can be modified.
   """
-  # Conditional styles are allowed
+  allowed_fields_to_change = {'rules', 'description'}
-  if updated == original or key == 'rules':
+  if updated == original or key in allowed_fields_to_change:
     return True
   elif key == 'widgetOptions':
     try:
@@ -201,18 +201,12 @@ def allowed_summary_change(key, updated, original):
       original_options = json.loads(original or '{}')
     except ValueError:
       return False
-    # Unfortunately all widgetOptions are allowed to change, except choice items. But it is
+    # Can do anything, but those options must be the same
-    # better to list those that can be changed.
+    must_be_the_same = {'choices', 'choiceOptions'}
-    # TODO: move choice items to separate column
-    allowed_to_change = {'widget', 'dateFormat', 'timeFormat', 'isCustomDateFormat', 'alignment',
-                         'fillColor', 'textColor', 'isCustomTimeFormat', 'isCustomDateFormat',
-                         'numMode', 'numSign', 'decimals', 'maxDecimals', 'currency',
-                         'fontBold', 'fontItalic', 'fontUnderline', 'fontStrikethrough',
-                         'rulesOptions'}
     # Helper function to remove protected keys from dictionary.
-    def trim(options):
+    def protected_options(options):
-      return {k: v for k, v in options.items() if k not in allowed_to_change}
+      return {k: v for k, v in options.items() if k in must_be_the_same}
-    return trim(updated_options) == trim(original_options)
+    return protected_options(updated_options) == protected_options(original_options)
   else:
     return False
 

test/nbrowser/DescriptionColumn.ts

@@ -9,11 +9,38 @@ describe('DescriptionColumn', function() {
   let session: gu.Session;
 
   before(async () => {
-    session = await gu.session().teamSite.login();
+    session = await gu.session().login();
+    await session.tempNewDoc(cleanup);
+  });
+
+  it('should allow to edit description on summary table', async () => {
+    await gu.toggleSidePanel('left', 'close');
+    // Add summary table.
+    await gu.addNewSection('Table', 'Table1', {summarize: ['A']});
+    await gu.sendActions([
+      ['AddRecord', 'Table1', null, {A: 1}],
+    ]);
+
+    // Set description on A column and count column.
+    await gu.openColumnPanel('A');
+    await getDescriptionInput().sendKeys('testA');
+    await gu.openColumnPanel('count');
+    await gu.waitForServer();
+    await gu.checkForErrors();
+    await getDescriptionInput().sendKeys('testCount');
+    await gu.openColumnPanel('A');
+    await gu.waitForServer();
+    await gu.checkForErrors();
+
+    await gu.reloadDoc();
+    await gu.openColumnPanel('A');
+    assert.equal(await getDescriptionInput().getAttribute('value'), 'testA');
+    await gu.openColumnPanel('count');
+    assert.equal(await getDescriptionInput().getAttribute('value'), 'testCount');
+    await gu.undo(4);
   });
 
   it('should switch between close and save', async () => {
-    await session.tempNewDoc(cleanup);
     // Add new column.
     await addColumn();
 
@@ -100,7 +127,6 @@ describe('DescriptionColumn', function() {
     assert.isFalse(await gu.getColumnHeader({col: 'D'}).isPresent());
   });
 
-
   it('shows links in the column description', async () => {
     const revert = await gu.begin();
 

test/nbrowser/HeaderColor.ts

@@ -5,22 +5,50 @@ import { setupTestSuite } from 'test/nbrowser/testUtils';
 const defaultHeaderBackgroundColor = '#f7f7f7';
 
 describe('HeaderColor', function () {
-  this.timeout(20000);
+  this.timeout('20s');
   const cleanup = setupTestSuite();
 
   before(async () => {
     // Create a new document
     const mainSession = await gu.session().login();
     await mainSession.tempNewDoc(cleanup, 'HeaderColor');
+  });
+
+  it('allows setting header colors in summary table', async function () {
+    const revert = await gu.begin();
+    await gu.toggleSidePanel('left', 'close');
+    // Add summary table.
+    await gu.addNewSection('Table', 'Table1', {summarize: ['A']});
+    await gu.sendActions([
+      ['AddRecord', 'Table1', null, {A: 1}],
+    ]);
+    await gu.toggleSidePanel('right', 'open');
+    const testStyle = async () => {
+      await gu.openHeaderColorPicker();
+      await gu.setFillColor('red');
+      await gu.setTextColor('blue');
+      await gu.applyStyle();
+      await gu.checkForErrors();
+      await gu.assertHeaderFillColor(await gu.getSelectedColumn(), 'red');
+      await gu.assertHeaderTextColor(await gu.getSelectedColumn(), 'blue');
+    };
+    await gu.openColumnPanel('A');
+    await testStyle();
+    await gu.openColumnPanel('count');
+    await testStyle();
+
+    await gu.waitForServer();
+    await revert();
+  });
+
+  it('should save by clicking away', async function () {
     // add records
     await gu.enterCell('a');
     await gu.enterCell('b');
     await gu.enterCell('c');
     await gu.toggleSidePanel('right', 'open');
     await driver.find('.test-right-tab-field').click();
-  });
 
-  it('should save by clicking away', async function () {
     await gu.getCell('A', 1).click();
     // open color picker
     await gu.openHeaderColorPicker();

test/nbrowser/gristUtils.ts

@@ -563,6 +563,10 @@ export function getColumnHeader(colOrColOptions: string|IColHeader): WebElementP
     sectionElem.findContent('.column_name .kf_elabel_text', exactMatch(col)).findClosest('.column_name'));
 }
 
+export function getSelectedColumn() {
+  return driver.find('.active_section .column_name.selected');
+}
+
 export async function getColumnNames() {
   const section = await driver.findWait('.active_section', 4000);
   return (await section.findAll('.column_name', el => el.getText()))
@@ -2439,6 +2443,11 @@ export function setFillColor(color: string) {
   return setColor(driver.find('.test-fill-input'), color);
 }
 
+export async function applyStyle() {
+  await driver.find('.test-colors-save').click();
+  await waitForServer();
+}
+
 export function getStyleRuleAt(nr: number) {
   return driver.find(`.test-widget-style-conditional-rule-${nr}`);
 }
@@ -2482,14 +2491,17 @@ export function openHeaderColorPicker() {
   return driver.find('.test-header-color-select .test-color-select').click();
 }
 
-export async function assertHeaderTextColor(col: string, color: string) {
+export async function assertHeaderTextColor(col: string|WebElement, color: string) {
-  await assertTextColor(await getColumnHeader(col), color);
+  const element = typeof col === 'string' ? await getColumnHeader(col) : col;
+  await assertTextColor(element, color);
 }
 
-export async function assertHeaderFillColor(col: string, color: string) {
+export async function assertHeaderFillColor(col: string|WebElement, color: string) {
-  await assertFillColor(await getColumnHeader(col), color);
+  const element = typeof col === 'string' ? await getColumnHeader(col) : col;
+  await assertFillColor(element, color);
 }
 
+
 /**
  * Opens a cell color picker, either the default one or the one for a specific style rule.
  */