From b504629cc0146ab942a54fdea3299e0b0896f7d3 Mon Sep 17 00:00:00 2001 From: Angelo Haller Date: Tue, 25 Aug 2020 10:57:04 -0500 Subject: [PATCH 1/2] Fix table editing redraw code on all platforms. This fixes bugs on all platforms not calling uiTableModelRowChanged() when setting a new value in edit mode. This is now automatically done in uiprivTableModelSetValue() so that ALL uiTable views are informed about the update. Darwin and windows did some custom redrawing which hid this bug. Unix does frequent unrelated redraw which hide the bug. It can often be experienced when double clicking a checkbox. --- common/tablemodel.c | 8 ++++++++ darwin/tablecolumn.m | 7 ------- windows/tableediting.cpp | 10 ---------- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/common/tablemodel.c b/common/tablemodel.c index dbda4a814..6a1c26721 100644 --- a/common/tablemodel.c +++ b/common/tablemodel.c @@ -41,6 +41,14 @@ void uiprivTableModelSetCellValue(uiTableModel *m, int row, int column, const ui mh = uiprivTableModelHandler(m); (*(mh->SetCellValue))(mh, m, row, column, value); + + // Abort redraw for button columns which signal being clicked + // by setting NULL. + // TODO check if NULL is never a valid value otherwise + if (value == NULL) + return; + + uiTableModelRowChanged(m, row); } const uiTableTextColumnOptionalParams uiprivDefaultTextColumnOptionalParams = { diff --git a/darwin/tablecolumn.m b/darwin/tablecolumn.m index 5038cc6bd..032ee7110 100644 --- a/darwin/tablecolumn.m +++ b/darwin/tablecolumn.m @@ -292,9 +292,6 @@ - (IBAction)uiprivOnTextFieldAction:(id)sender value = uiNewTableValueString([[self->tf stringValue] UTF8String]); uiprivTableModelSetCellValue(self->m, row, self->textModelColumn, value); uiFreeTableValue(value); - // always refresh the value in case the model rejected it - // TODO document that we do this, but not for the whole row (or decide to do both, or do neither...) - [self uiprivUpdate:row]; } - (IBAction)uiprivOnCheckboxAction:(id)sender @@ -306,8 +303,6 @@ - (IBAction)uiprivOnCheckboxAction:(id)sender value = uiNewTableValueInt([self->cb state] != NSOffState); uiprivTableModelSetCellValue(self->m, row, self->checkboxModelColumn, value); uiFreeTableValue(value); - // always refresh the value in case the model rejected it - [self uiprivUpdate:row]; } @end @@ -539,8 +534,6 @@ - (IBAction)uiprivOnClicked:(id)sender row = [self->t->tv rowForView:self->b]; uiprivTableModelSetCellValue(self->m, row, self->modelColumn, NULL); - // TODO document we DON'T update the cell after doing this - // TODO or decide what to do instead } @end diff --git a/windows/tableediting.cpp b/windows/tableediting.cpp index 7bbb450c2..281d1370a 100644 --- a/windows/tableediting.cpp +++ b/windows/tableediting.cpp @@ -168,11 +168,6 @@ HRESULT uiprivTableFinishEditingText(uiTable *t) p = (*(t->columns))[t->editedSubitem]; uiprivTableModelSetCellValue(t->model, t->editedItem, p->textModelColumn, value); uiFreeTableValue(value); - // always refresh the value in case the model rejected it - if (SendMessageW(t->hwnd, LVM_UPDATE, (WPARAM) (t->editedItem), 0) == (LRESULT) (-1)) { - logLastError(L"LVM_UPDATE"); - return E_FAIL; - } return uiprivTableAbortEditingText(t); } @@ -250,11 +245,6 @@ HRESULT uiprivTableHandleNM_CLICK(uiTable *t, NMITEMACTIVATE *nm, LRESULT *lResu uiFreeTableValue(value); } else uiprivTableModelSetCellValue(t->model, ht.iItem, modelColumn, NULL); - // always refresh the value in case the model rejected it - if (SendMessageW(t->hwnd, LVM_UPDATE, (WPARAM) (ht.iItem), 0) == (LRESULT) (-1)) { - logLastError(L"LVM_UPDATE"); - return E_FAIL; - } done: *lResult = 0; From 772bfad79f126b7e8ca33c6776aaaa61e65ba6ba Mon Sep 17 00:00:00 2001 From: Angelo Haller Date: Thu, 27 Aug 2020 14:02:56 -0500 Subject: [PATCH 2/2] Remove early abort for NULL values in uiprivTableModelSetCellValue. We are changing the value, update the row! There are several uses for setting NULL values, IMO all abusing the API for different causes. Fix those API abuses instead: - This will cause one unneeded redraw when clicking on table column buttons as they signal a click through setting NULL. Possibly use a dedicated callback handler for that instead? - This will fix resetting colors to default, as this is signaled through a NULL value. --- common/tablemodel.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/common/tablemodel.c b/common/tablemodel.c index 6a1c26721..7fc9c215d 100644 --- a/common/tablemodel.c +++ b/common/tablemodel.c @@ -42,12 +42,6 @@ void uiprivTableModelSetCellValue(uiTableModel *m, int row, int column, const ui mh = uiprivTableModelHandler(m); (*(mh->SetCellValue))(mh, m, row, column, value); - // Abort redraw for button columns which signal being clicked - // by setting NULL. - // TODO check if NULL is never a valid value otherwise - if (value == NULL) - return; - uiTableModelRowChanged(m, row); }