From 2217eebaec97aa48c37c83feb8d7f97e7f58625a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89milie=20Feral?= Date: Fri, 26 Apr 2019 14:48:31 +0200 Subject: [PATCH] [escher] SelectableTableView: when reloading data, we temporary deselect the table. We warn the SelectableTableViewDelegate that the selection change is 'within a temporary selection change' when notifying it of the change. --- apps/calculation/history_controller.cpp | 4 ++-- apps/calculation/history_controller.h | 2 +- apps/code/console_controller.cpp | 5 ++++- apps/code/console_controller.h | 2 +- apps/code/menu_controller.cpp | 4 ++-- apps/code/menu_controller.h | 2 +- apps/home/controller.cpp | 5 ++++- apps/home/controller.h | 2 +- apps/regression/calculation_controller.cpp | 5 ++++- apps/regression/calculation_controller.h | 2 +- apps/sequence/list/list_parameter_controller.cpp | 4 ++-- apps/sequence/list/list_parameter_controller.h | 2 +- apps/shared/expression_model_list_controller.cpp | 4 ++-- apps/shared/expression_model_list_controller.h | 2 +- apps/shared/function_list_controller.cpp | 6 +++--- apps/shared/function_list_controller.h | 2 +- escher/include/escher/selectable_table_view.h | 4 ++-- .../escher/selectable_table_view_delegate.h | 7 ++++++- escher/src/selectable_table_view.cpp | 16 ++++++++-------- escher/src/selectable_table_view_delegate.cpp | 2 +- 20 files changed, 48 insertions(+), 34 deletions(-) diff --git a/apps/calculation/history_controller.cpp b/apps/calculation/history_controller.cpp index 786345dfc..9678c533c 100644 --- a/apps/calculation/history_controller.cpp +++ b/apps/calculation/history_controller.cpp @@ -105,8 +105,8 @@ bool HistoryController::handleEvent(Ion::Events::Event event) { return false; } -void HistoryController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { - if (previousSelectedCellY == selectedRow()) { +void HistoryController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { + if (withinTemporarySelection || previousSelectedCellY == selectedRow()) { return; } HistoryViewCell * cell = static_cast(t->selectedCell()); diff --git a/apps/calculation/history_controller.h b/apps/calculation/history_controller.h index 9486748e5..67caf5ec9 100644 --- a/apps/calculation/history_controller.h +++ b/apps/calculation/history_controller.h @@ -24,7 +24,7 @@ public: void willDisplayCellForIndex(HighlightCell * cell, int index) override; KDCoordinate rowHeight(int j) override; int typeAtLocation(int i, int j) override; - void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) override; + void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection = false) override; void scrollToCell(int i, int j); private: CalculationSelectableTableView * selectableTableView(); diff --git a/apps/code/console_controller.cpp b/apps/code/console_controller.cpp index d5da9c826..429a0f170 100644 --- a/apps/code/console_controller.cpp +++ b/apps/code/console_controller.cpp @@ -245,7 +245,10 @@ void ConsoleController::willDisplayCellAtLocation(HighlightCell * cell, int i, i } } -void ConsoleController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { +void ConsoleController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { + if (withinTemporarySelection) { + return; + } if (t->selectedRow() == m_consoleStore.numberOfLines()) { m_editCell.setEditing(true); return; diff --git a/apps/code/console_controller.h b/apps/code/console_controller.h index d39f6edfe..43779f340 100644 --- a/apps/code/console_controller.h +++ b/apps/code/console_controller.h @@ -52,7 +52,7 @@ public: void willDisplayCellAtLocation(HighlightCell * cell, int i, int j) override; // SelectableTableViewDelegate - void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) override; + void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) override; // TextFieldDelegate bool textFieldShouldFinishEditing(TextField * textField, Ion::Events::Event event) override; diff --git a/apps/code/menu_controller.cpp b/apps/code/menu_controller.cpp index 60dda9eec..78adafc2f 100644 --- a/apps/code/menu_controller.cpp +++ b/apps/code/menu_controller.cpp @@ -272,8 +272,8 @@ void MenuController::willDisplayScriptTitleCellForIndex(HighlightCell * cell, in (static_cast(cell))->textField()->setText(m_scriptStore->scriptAtIndex(index).fullName()); } -void MenuController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { - if (selectedRow() == numberOfRows() - 1 && selectedColumn() == 1 && m_shouldDisplayAddScriptRow) { +void MenuController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { + if (!withinTemporarySelection && selectedRow() == numberOfRows() - 1 && selectedColumn() == 1 && m_shouldDisplayAddScriptRow) { t->selectCellAtLocation(0, numberOfRows()-1); } } diff --git a/apps/code/menu_controller.h b/apps/code/menu_controller.h index 7b94014f4..5848a302a 100644 --- a/apps/code/menu_controller.h +++ b/apps/code/menu_controller.h @@ -46,7 +46,7 @@ public: void willDisplayScriptTitleCellForIndex(HighlightCell * cell, int index); /* SelectableTableViewDelegate */ - void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) override; + void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) override; /* TextFieldDelegate */ bool textFieldShouldFinishEditing(TextField * textField, Ion::Events::Event event) override; diff --git a/apps/home/controller.cpp b/apps/home/controller.cpp index 10f0075a5..f177204ec 100644 --- a/apps/home/controller.cpp +++ b/apps/home/controller.cpp @@ -134,7 +134,10 @@ int Controller::numberOfIcons() { return m_container->numberOfApps() - 1; } -void Controller::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { +void Controller::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { + if (withinTemporarySelection) { + return; + } /* If the number of apps (including home) is != 3*n+1, when we display the * lowest icons, the other(s) are empty. As no icon is thus redrawn on the * previous ones, the cell is not cleaned. We need to redraw a white rect on diff --git a/apps/home/controller.h b/apps/home/controller.h index ae3be69e5..116b6291a 100644 --- a/apps/home/controller.h +++ b/apps/home/controller.h @@ -25,7 +25,7 @@ public: virtual HighlightCell * reusableCell(int index) override; virtual int reusableCellCount() override; void willDisplayCellAtLocation(HighlightCell * cell, int i, int j) override; - void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) override; + void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) override; private: int numberOfIcons(); class ContentView : public View { diff --git a/apps/regression/calculation_controller.cpp b/apps/regression/calculation_controller.cpp index 0a0ddb1db..145950d19 100644 --- a/apps/regression/calculation_controller.cpp +++ b/apps/regression/calculation_controller.cpp @@ -65,7 +65,10 @@ void CalculationController::didBecomeFirstResponder() { TabTableController::didBecomeFirstResponder(); } -void CalculationController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { +void CalculationController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { + if (withinTemporarySelection) { + return; + } /* To prevent selecting cell with no content (top left corner of the table), * as soon as the selected cell is the top left corner, we either reselect * the previous cell or select the tab controller depending on from which cell diff --git a/apps/regression/calculation_controller.h b/apps/regression/calculation_controller.h index e59265a62..00237fa73 100644 --- a/apps/regression/calculation_controller.h +++ b/apps/regression/calculation_controller.h @@ -29,7 +29,7 @@ public: void didBecomeFirstResponder() override; // SelectableTableViewDelegate - void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) override; + void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) override; // AlternateEmptyViewDefaultDelegate bool isEmpty() const override; diff --git a/apps/sequence/list/list_parameter_controller.cpp b/apps/sequence/list/list_parameter_controller.cpp index cc0f25fcf..ac36094be 100644 --- a/apps/sequence/list/list_parameter_controller.cpp +++ b/apps/sequence/list/list_parameter_controller.cpp @@ -92,8 +92,8 @@ bool ListParameterController::textFieldDidFinishEditing(TextField * textField, c return true; } -void ListParameterController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { - if (previousSelectedCellX == t->selectedColumn() && previousSelectedCellY == t->selectedRow()) { +void ListParameterController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { + if (withinTemporarySelection || (previousSelectedCellX == t->selectedColumn() && previousSelectedCellY == t->selectedRow())) { return; } if (!hasInitialRankRow()) { diff --git a/apps/sequence/list/list_parameter_controller.h b/apps/sequence/list/list_parameter_controller.h index 09ff8c4cd..e43ee5d43 100644 --- a/apps/sequence/list/list_parameter_controller.h +++ b/apps/sequence/list/list_parameter_controller.h @@ -19,7 +19,7 @@ public: bool textFieldShouldFinishEditing(TextField * textField, Ion::Events::Event event) override; bool textFieldDidFinishEditing(TextField * textField, const char * text, Ion::Events::Event event) override; - void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) override; + void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) override; Shared::TextFieldDelegateApp * textFieldDelegateApp() override; // ListViewDataSource diff --git a/apps/shared/expression_model_list_controller.cpp b/apps/shared/expression_model_list_controller.cpp index fe4a33aa4..1bca31721 100644 --- a/apps/shared/expression_model_list_controller.cpp +++ b/apps/shared/expression_model_list_controller.cpp @@ -16,11 +16,11 @@ ExpressionModelListController::ExpressionModelListController(Responder * parentR m_addNewModel.setMessage(text); } -void ExpressionModelListController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { +void ExpressionModelListController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { int currentSelectedRow = selectedRow(); // Update m_cumulatedHeightForSelectedIndex if we scrolled one cell up/down - if (previousSelectedCellY >= 0 && previousSelectedCellY == previousSelectedCellY + 1) { + if (currentSelectedRow >= 0 && currentSelectedRow == previousSelectedCellY + 1) { /* We selected the cell under the previous cell. Shift the memoized cell * heights. */ shiftMemoization(true); diff --git a/apps/shared/expression_model_list_controller.h b/apps/shared/expression_model_list_controller.h index 9fc00367f..82ae4db1a 100644 --- a/apps/shared/expression_model_list_controller.h +++ b/apps/shared/expression_model_list_controller.h @@ -13,7 +13,7 @@ public: protected: static constexpr KDCoordinate k_expressionMargin = 5; // SelectableTableViewDelegate - void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) override; + void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) override; // TableViewDataSource virtual int numberOfExpressionRows(); KDCoordinate memoizedRowHeight(int j); diff --git a/apps/shared/function_list_controller.cpp b/apps/shared/function_list_controller.cpp index cc65e5211..9dfa8dce2 100644 --- a/apps/shared/function_list_controller.cpp +++ b/apps/shared/function_list_controller.cpp @@ -214,11 +214,11 @@ void FunctionListController::willExitResponderChain(Responder * nextFirstRespond /* SelectableTableViewDelegate */ -void FunctionListController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { +void FunctionListController::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { // Update memoization of cell heights - ExpressionModelListController::tableViewDidChangeSelection(t, previousSelectedCellX, previousSelectedCellY); + ExpressionModelListController::tableViewDidChangeSelection(t, previousSelectedCellX, previousSelectedCellY, withinTemporarySelection); // Do not select the cell left of the "addEmptyFunction" cell - if (isAddEmptyRow(selectedRow()) && selectedColumn() == 0) { + if (!withinTemporarySelection && isAddEmptyRow(selectedRow()) && selectedColumn() == 0) { t->selectCellAtLocation(1, numberOfRows()-1); } } diff --git a/apps/shared/function_list_controller.h b/apps/shared/function_list_controller.h index 273edd401..8bc0dd055 100644 --- a/apps/shared/function_list_controller.h +++ b/apps/shared/function_list_controller.h @@ -45,7 +45,7 @@ public: View * view() override { return &m_selectableTableView; } /* SelectableTableViewDelegate*/ - void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) override; + void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) override; /* ExpressionModelListController */ SelectableTableView * selectableTableView() override { return &m_selectableTableView; } diff --git a/escher/include/escher/selectable_table_view.h b/escher/include/escher/selectable_table_view.h index 6e1e7c000..cecd44a74 100644 --- a/escher/include/escher/selectable_table_view.h +++ b/escher/include/escher/selectable_table_view.h @@ -29,8 +29,8 @@ public: virtual bool handleEvent(Ion::Events::Event event) override; virtual void didEnterResponderChain(Responder * previousFirstResponder) override; virtual void willExitResponderChain(Responder * nextFirstResponder) override; - void deselectTable(bool notifySelectableDelegate = true); - bool selectCellAtLocation(int i, int j, bool setFirstResponder = true, bool notifySelectableDelegate = true); + void deselectTable(bool withinTemporarySelection = false); + bool selectCellAtLocation(int i, int j, bool setFirstResponder = true, bool withinTemporarySelection = false); HighlightCell * selectedCell(); protected: SelectableTableViewDataSource * m_selectionDataSource; diff --git a/escher/include/escher/selectable_table_view_delegate.h b/escher/include/escher/selectable_table_view_delegate.h index 33249f3f9..e8f11864a 100644 --- a/escher/include/escher/selectable_table_view_delegate.h +++ b/escher/include/escher/selectable_table_view_delegate.h @@ -5,7 +5,12 @@ class SelectableTableView; class SelectableTableViewDelegate { public: - virtual void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY); + /* withinTemporarySelection flag indicates when the selection change happens + * in a temporary deselection: indeed, when reloading the data of the table, + * we deselect the table before re-layouting the entire table and re-select + * the previous selected cell. We might implement different course of action + * when the selection change is 'real' or within temporary selection. */ + virtual void tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection = false); }; #endif diff --git a/escher/src/selectable_table_view.cpp b/escher/src/selectable_table_view.cpp index 995e7c3e7..226701a6d 100644 --- a/escher/src/selectable_table_view.cpp +++ b/escher/src/selectable_table_view.cpp @@ -37,7 +37,7 @@ void SelectableTableView::selectColumn(int i) { void SelectableTableView::reloadData(bool setFirstResponder) { int col = selectedColumn(); int row = selectedRow(); - deselectTable(false); + deselectTable(true); /* FIXME: The problem with calling deselectTable is that at this point in time * the datasource's model is very likely to have changed. Therefore it's * rather complicated to get a pointer to the currently selected cell (in @@ -45,7 +45,7 @@ void SelectableTableView::reloadData(bool setFirstResponder) { /* As a workaround, datasources can reset the highlighted state in their * willDisplayCell callback. */ TableView::layoutSubviews(); - selectCellAtLocation(col, row, setFirstResponder, false); + selectCellAtLocation(col, row, setFirstResponder, true); } void SelectableTableView::didEnterResponderChain(Responder * previousFirstResponder) { @@ -60,18 +60,18 @@ void SelectableTableView::willExitResponderChain(Responder * nextFirstResponder) unhighlightSelectedCell(); } -void SelectableTableView::deselectTable(bool notifySelectableDelegate) { +void SelectableTableView::deselectTable(bool withinTemporarySelection) { unhighlightSelectedCell(); int previousSelectedCellX = selectedColumn(); int previousSelectedCellY = selectedRow(); selectColumn(0); selectRow(-1); - if (m_delegate && notifySelectableDelegate) { - m_delegate->tableViewDidChangeSelection(this, previousSelectedCellX, previousSelectedCellY); + if (m_delegate) { + m_delegate->tableViewDidChangeSelection(this, previousSelectedCellX, previousSelectedCellY, withinTemporarySelection); } } -bool SelectableTableView::selectCellAtLocation(int i, int j, bool setFirstResponder, bool notifySelectableDelegate) { +bool SelectableTableView::selectCellAtLocation(int i, int j, bool setFirstResponder, bool withinTemporarySelection) { if (i < 0 || i >= dataSource()->numberOfColumns()) { return false; } @@ -84,8 +84,8 @@ bool SelectableTableView::selectCellAtLocation(int i, int j, bool setFirstRespon selectColumn(i); selectRow(j); - if (m_delegate && notifySelectableDelegate) { - m_delegate->tableViewDidChangeSelection(this, previousX, previousY); + if (m_delegate) { + m_delegate->tableViewDidChangeSelection(this, previousX, previousY, withinTemporarySelection); } /* We need to scroll: diff --git a/escher/src/selectable_table_view_delegate.cpp b/escher/src/selectable_table_view_delegate.cpp index 3a8de3636..25da1557a 100644 --- a/escher/src/selectable_table_view_delegate.cpp +++ b/escher/src/selectable_table_view_delegate.cpp @@ -1,4 +1,4 @@ #include -void SelectableTableViewDelegate::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY) { +void SelectableTableViewDelegate::tableViewDidChangeSelection(SelectableTableView * t, int previousSelectedCellX, int previousSelectedCellY, bool withinTemporarySelection) { }