From 5b26c84f0df25ef771819c65ec919dda4280d58a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Fri, 13 Sep 2024 16:02:11 +0200 Subject: [PATCH] (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 | 2 +- sandbox/grist/test_summary.py | 38 +++++++++++++++++------------- sandbox/grist/useractions.py | 20 ++++++---------- test/nbrowser/DescriptionColumn.ts | 32 ++++++++++++++++++++++--- test/nbrowser/HeaderColor.ts | 34 +++++++++++++++++++++++--- test/nbrowser/gristUtils.ts | 20 ++++++++++++---- 6 files changed, 105 insertions(+), 41 deletions(-) diff --git a/app/client/models/NotifyModel.ts b/app/client/models/NotifyModel.ts index c3af0b37..ea4221f0 100644 --- a/app/client/models/NotifyModel.ts +++ b/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')) ) ), diff --git a/sandbox/grist/test_summary.py b/sandbox/grist/test_summary.py index 6af9e2a8..3787cf88 100644 --- a/sandbox/grist/test_summary.py +++ b/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. - new = '{"widget":"TextBox","cant":"center"}' - old = '{"widget":"Spinner","cant":"center"}' + # Can update but must leave choices options. + new = '{"widget":"TextBox","choices":"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"}' - old = '{"widget":"TextBox","cant":"old"}' + new = '{"widget":"TextBox","choices":"new"}' + 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}}' - old = '{"widget":"TextBox","alignment":{"prop":2},"cant":{"prop":1}}' + new = '{"widget":"TextBox","alignment":{"prop":1},"choices":{"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}}' - old = '{"widget":"TextBox","cant":{"prop":2}}' + new = '{"widget":"TextBox","choices":{"prop":1}}' + old = '{"widget":"TextBox","choices":{"prop":2}}' self.assertFalse(widgetOptions(new, old)) # Can't update lists - new = '{"widget":"TextBox","cant":[1, 2]}' - old = '{"widget":"TextBox","cant":[2, 1]}' + new = '{"widget":"TextBox","choices":[1, 2]}' + 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]}' - old = '{"widget":"Spinner","dontChange":[1, 2]}' + new = '{"widget":"TextBox","choices":[1, 2]}' + old = '{"widget":"Spinner","choices":[1, 2]}' self.assertTrue(widgetOptions(new, old)) # pylint: enable=R0915 + +if __name__ == "__main__": + unittest.main() diff --git a/sandbox/grist/useractions.py b/sandbox/grist/useractions.py index 7801af11..d62f1ee4 100644 --- a/sandbox/grist/useractions.py +++ b/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 - if updated == original or key == 'rules': + allowed_fields_to_change = {'rules', 'description'} + 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 - # better to list those that can be changed. - # 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'} + # Can do anything, but those options must be the same + must_be_the_same = {'choices', 'choiceOptions'} # Helper function to remove protected keys from dictionary. - def trim(options): - return {k: v for k, v in options.items() if k not in allowed_to_change} - return trim(updated_options) == trim(original_options) + def protected_options(options): + return {k: v for k, v in options.items() if k in must_be_the_same} + return protected_options(updated_options) == protected_options(original_options) else: return False diff --git a/test/nbrowser/DescriptionColumn.ts b/test/nbrowser/DescriptionColumn.ts index cd14102e..aa7dc57c 100644 --- a/test/nbrowser/DescriptionColumn.ts +++ b/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(); diff --git a/test/nbrowser/HeaderColor.ts b/test/nbrowser/HeaderColor.ts index 29ef6f32..d733c437 100644 --- a/test/nbrowser/HeaderColor.ts +++ b/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(); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 7cfa14a6..26e112aa 100644 --- a/test/nbrowser/gristUtils.ts +++ b/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) { - await assertTextColor(await getColumnHeader(col), color); +export async function assertHeaderTextColor(col: string|WebElement, color: string) { + const element = typeof col === 'string' ? await getColumnHeader(col) : col; + await assertTextColor(element, color); } -export async function assertHeaderFillColor(col: string, color: string) { - await assertFillColor(await getColumnHeader(col), color); +export async function assertHeaderFillColor(col: string|WebElement, color: string) { + 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. */