First pass at keyboard navigation (currently only for CellTable) Review at http://gwt-code-reviews.appspot.com/710802 Review by: jlabanca@google.com git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8488 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/cell/client/AbstractCell.java b/user/src/com/google/gwt/cell/client/AbstractCell.java index 288cb5a..b1f8f2b 100644 --- a/user/src/com/google/gwt/cell/client/AbstractCell.java +++ b/user/src/com/google/gwt/cell/client/AbstractCell.java
@@ -77,6 +77,14 @@ } /** + * Returns false. Subclasses that support editing should override this method + * to return the current editing status. + */ + public boolean isEditing(Element element, C value, Object key) { + return false; + } + + /** * {@inheritDoc} * * This method is a no-op in {@link AbstractCell}. If you override this method
diff --git a/user/src/com/google/gwt/cell/client/Cell.java b/user/src/com/google/gwt/cell/client/Cell.java index 7e3b974..96d5310 100644 --- a/user/src/com/google/gwt/cell/client/Cell.java +++ b/user/src/com/google/gwt/cell/client/Cell.java
@@ -62,6 +62,18 @@ boolean handlesSelection(); /** + * Returns true if the cell is currently editing the data identified by the + * given element and key. While a cell is editing, widgets containing the cell + * may chooses to pass keystrokes directly to the cell rather than using them + * for navigation purposes. + * + * @param parent the parent Element + * @param value the value associated with the cell + * @param key the unique key associated with the row object + */ + boolean isEditing(Element parent, C value, Object key); + + /** * Handle a browser event that took place within the cell. The default * implementation returns null. * @@ -77,7 +89,6 @@ /** * Render a cell as HTML into a StringBuilder, suitable for passing to * {@link Element#setInnerHTML} on a container element. - * * @param value the cell value to be rendered * @param key the unique key associated with the row object * @param sb the StringBuilder to be written to
diff --git a/user/src/com/google/gwt/cell/client/CheckboxCell.java b/user/src/com/google/gwt/cell/client/CheckboxCell.java index 7aa6edc..8839a24 100644 --- a/user/src/com/google/gwt/cell/client/CheckboxCell.java +++ b/user/src/com/google/gwt/cell/client/CheckboxCell.java
@@ -18,9 +18,11 @@ import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.InputElement; import com.google.gwt.dom.client.NativeEvent; +import com.google.gwt.event.dom.client.KeyCodes; /** - * A {@link Cell} used to render a checkbox. + * A {@link Cell} used to render a checkbox. The value of the checkbox + * may be toggled using the ENTER key as well as via mouse click. * * <p> * Note: This class is new and its interface subject to change. @@ -43,7 +45,7 @@ * @param isSelectBox true if the cell controls the selection state */ public CheckboxCell(boolean isSelectBox) { - super("change"); + super("change", "keyup"); this.isSelectBox = isSelectBox; } @@ -61,9 +63,19 @@ public void onBrowserEvent(Element parent, Boolean value, Object key, NativeEvent event, ValueUpdater<Boolean> valueUpdater) { String type = event.getType(); - if ("change".equals(type)) { + + boolean enterPressed = "keyup".equals(type) && + event.getKeyCode() == KeyCodes.KEY_ENTER; + if ("change".equals(type) || enterPressed) { InputElement input = parent.getFirstChild().cast(); Boolean isChecked = input.isChecked(); + + // If the enter key was pressed, toggle the value + if (enterPressed) { + isChecked = !isChecked; + input.setChecked(isChecked); + } + setViewData(key, isChecked); if (valueUpdater != null) { valueUpdater.update(isChecked);
diff --git a/user/src/com/google/gwt/cell/client/EditTextCell.java b/user/src/com/google/gwt/cell/client/EditTextCell.java index 7bfaea5..44c3874 100644 --- a/user/src/com/google/gwt/cell/client/EditTextCell.java +++ b/user/src/com/google/gwt/cell/client/EditTextCell.java
@@ -123,7 +123,12 @@ } public EditTextCell() { - super("click", "keydown", "keyup", "blur"); + super("click", "keyup", "keydown", "blur"); + } + + public boolean isEditing(Element element, String value, Object key) { + ViewData viewData = getViewData(key); + return viewData == null ? false : viewData.isEditing(); } @Override @@ -133,15 +138,21 @@ if (viewData != null && viewData.isEditing()) { // Handle the edit event. editEvent(parent, key, viewData, event, valueUpdater); - } else if ("click".equals(event.getType())) { - // Go into edit mode. - if (viewData == null) { - viewData = new ViewData(value); - setViewData(key, viewData); - } else { - viewData.setEditing(true); + } else { + String type = event.getType(); + int keyCode = event.getKeyCode(); + boolean enterPressed = "keyup".equals(type) && + keyCode == KeyCodes.KEY_ENTER; + if ("click".equals(type) || enterPressed) { + // Go into edit mode. + if (viewData == null) { + viewData = new ViewData(value); + setViewData(key, viewData); + } else { + viewData.setEditing(true); + } + edit(parent, value, key); } - edit(parent, value, key); } } @@ -209,12 +220,14 @@ private void editEvent(Element parent, Object key, ViewData viewData, NativeEvent event, ValueUpdater<String> valueUpdater) { String type = event.getType(); - if ("keydown".equals(type)) { + boolean keyUp = "keyup".equals(type); + boolean keyDown = "keydown".equals(type); + if (keyUp || keyDown) { int keyCode = event.getKeyCode(); - if (keyCode == KeyCodes.KEY_ENTER) { + if (keyUp && keyCode == KeyCodes.KEY_ENTER) { // Commit the change. commit(parent, viewData, valueUpdater); - } else if (keyCode == KeyCodes.KEY_ESCAPE) { + } else if (keyUp && keyCode == KeyCodes.KEY_ESCAPE) { // Cancel edit mode. String originalText = viewData.getOriginal(); if (viewData.isEditingAgain()) { @@ -224,10 +237,10 @@ setViewData(key, null); } cancel(parent, originalText); + } else { + // Update the text in the view data on each key. + updateViewData(parent, viewData, true); } - } else if ("keyup".equals(type)) { - // Update the text in the view data on each key. - updateViewData(parent, viewData, true); } else if ("blur".equals(type)) { // Commit the change. Ensure that we are blurring the input element and // not the parent element itself.
diff --git a/user/src/com/google/gwt/cell/client/IconCellDecorator.java b/user/src/com/google/gwt/cell/client/IconCellDecorator.java index f2575a5..3a1bb92 100644 --- a/user/src/com/google/gwt/cell/client/IconCellDecorator.java +++ b/user/src/com/google/gwt/cell/client/IconCellDecorator.java
@@ -77,6 +77,10 @@ public boolean handlesSelection() { return cell.handlesSelection(); } + + public boolean isEditing(Element element, C value, Object key) { + return cell.isEditing(element, value, key); + } public void onBrowserEvent(Element parent, C value, Object key, NativeEvent event, ValueUpdater<C> valueUpdater) {
diff --git a/user/src/com/google/gwt/user/cellview/client/CellBrowser.java b/user/src/com/google/gwt/user/cellview/client/CellBrowser.java index 52b7196..7d2c3a3 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellBrowser.java +++ b/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
@@ -205,6 +205,10 @@ public boolean handlesSelection() { return cell.handlesSelection(); } + + public boolean isEditing(Element element, C value, Object key) { + return cell.isEditing(element, value, key); + } public void onBrowserEvent(Element parent, C value, Object key, NativeEvent event, ValueUpdater<C> valueUpdater) {
diff --git a/user/src/com/google/gwt/user/cellview/client/CellList.java b/user/src/com/google/gwt/user/cellview/client/CellList.java index ec4e915..bc17dec 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellList.java +++ b/user/src/com/google/gwt/user/cellview/client/CellList.java
@@ -122,6 +122,9 @@ } } + public void resetFocus() { + } + public void setLoadingState(LoadingState state) { showOrHide(emptyMessageElem, state == LoadingState.EMPTY); // TODO(jlabanca): Add a loading icon.
diff --git a/user/src/com/google/gwt/user/cellview/client/CellTable.java b/user/src/com/google/gwt/user/cellview/client/CellTable.java index 5678e35..ce3ccae 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellTable.java +++ b/user/src/com/google/gwt/user/cellview/client/CellTable.java
@@ -27,6 +27,7 @@ import com.google.gwt.dom.client.TableElement; import com.google.gwt.dom.client.TableRowElement; import com.google.gwt.dom.client.TableSectionElement; +import com.google.gwt.event.dom.client.KeyCodes; import com.google.gwt.resources.client.ClientBundle; import com.google.gwt.resources.client.CssResource; import com.google.gwt.resources.client.ImageResource; @@ -161,6 +162,16 @@ String hoveredRow(); /** + * Applied to the keyboard selected cell. + */ + String keyboardSelectedCell(); + + /** + * Applied to the keyboard selected row. + */ + String keyboardSelectedRow(); + + /** * Applied to the last column. */ String lastColumn(); @@ -381,6 +392,19 @@ TABLE_IMPL.replaceAllRows( CellTable.this, tbody, CellBasedWidgetImpl.get().processHtml(html)); } + + public void resetFocus() { + int pageStart = getPageStart(); + int offset = keyboardSelectedRow - pageStart; + if (offset >= 0 && offset < getPageSize()) { + TableRowElement tr = getRowElement(offset); + TableCellElement td = tr.getCells().getItem(keyboardSelectedColumn); + tr.addClassName(style.keyboardSelectedRow()); + td.addClassName(style.keyboardSelectedCell()); + td.setTabIndex(0); + td.focus(); // TODO (rice) only focus if we were focused previously + } + } public void setLoadingState(LoadingState state) { setLoadingIconVisible(state == LoadingState.LOADING); @@ -416,11 +440,17 @@ return DEFAULT_RESOURCES; } + private boolean cellIsEditing; private final TableColElement colgroup; + private List<Column<T, ?>> columns = new ArrayList<Column<T, ?>>(); + private boolean dependsOnSelection; + private List<Header<?>> footers = new ArrayList<Header<?>>(); + private boolean handlesSelection; + private List<Header<?>> headers = new ArrayList<Header<?>>(); /** @@ -430,6 +460,10 @@ private TableRowElement hoveringRow; + private int keyboardSelectedColumn = 0; + + private int keyboardSelectedRow = 0; + /** * The presenter. */ @@ -504,6 +538,7 @@ this.style.ensureInjected(); setElement(table = Document.get().createTableElement()); + table.setCellSpacing(0); colgroup = Document.get().createColGroupElement(); table.appendChild(colgroup); @@ -530,7 +565,8 @@ setPageSize(pageSize); // Sink events. - sinkEvents(Event.ONCLICK | Event.ONMOUSEOVER | Event.ONMOUSEOUT); + sinkEvents(Event.ONCLICK | Event.ONMOUSEOVER | Event.ONMOUSEOUT | + Event.ONKEYUP | Event.ONKEYDOWN); } /** @@ -670,14 +706,28 @@ public void onBrowserEvent(Event event) { CellBasedWidgetImpl.get().onBrowserEvent(this, event); super.onBrowserEvent(event); - + + String eventType = event.getType(); + boolean keyUp = "keyup".equals(eventType); + boolean keyDown = "keydown".equals(eventType); + + // Ignore keydown events unless the cell is in edit mode + if (keyDown && !cellIsEditing) { + return; + } + if (keyUp && !cellIsEditing) { + if (handleKey(event)) { + return; + } + } + // Find the cell where the event occurred. EventTarget eventTarget = event.getEventTarget(); - TableCellElement cell = null; + TableCellElement tableCell = null; if (eventTarget != null && Element.is(eventTarget)) { - cell = findNearestParentCell(Element.as(eventTarget)); + tableCell = findNearestParentCell(Element.as(eventTarget)); } - if (cell == null) { + if (tableCell == null) { return; } @@ -685,7 +735,7 @@ // the table has been refreshed before the current event fired (ex. change // event refreshes before mouseup fires), so we need to check each parent // element. - Element trElem = cell.getParentElement(); + Element trElem = tableCell.getParentElement(); if (trElem == null) { return; } @@ -697,19 +747,18 @@ TableSectionElement section = TableSectionElement.as(sectionElem); // Forward the event to the associated header, footer, or column. - String eventType = event.getType(); - int col = cell.getCellIndex(); + int col = tableCell.getCellIndex(); if (section == thead) { Header<?> header = headers.get(col); if (header != null && cellConsumesEventType(header.getCell(), eventType)) { - header.onBrowserEvent(cell, event); + header.onBrowserEvent(tableCell, event); } } else if (section == tfoot) { Header<?> footer = footers.get(col); if (footer != null && cellConsumesEventType(footer.getCell(), eventType)) { - footer.onBrowserEvent(cell, event); + footer.onBrowserEvent(tableCell, event); } } else if (section == tbody) { // Update the hover state. @@ -724,22 +773,17 @@ hoveringRow = null; tr.removeClassName(style.hoveredRow()); } - + // Update selection. Selection occurs before firing the event to the cell // in case the cell operates on the currently selected item. T value = presenter.getData().get(row); SelectionModel<? super T> selectionModel = presenter.getSelectionModel(); - Column<T, ?> column = columns.get(col); if (selectionModel != null && "click".equals(eventType) && !handlesSelection) { selectionModel.setSelected(value, true); } - - // Fire the event to the cell. - if (cellConsumesEventType(column.getCell(), eventType)) { - column.onBrowserEvent( - cell, getPageStart() + row, value, event, providesKey); - } + + fireEventToCell(event, eventType, tableCell, value, col, row); } } @@ -764,6 +808,20 @@ /** * Remove a column. * + * @param col the column to remove + */ + public void removeColumn(Column<T, ?> col) { + int index = columns.indexOf(col); + if (index < 0) { + throw new IllegalArgumentException( + "The specified column is not part of this table."); + } + removeColumn(index); + } + + /** + * Remove a column. + * * @param index the column index */ public void removeColumn(int index) { @@ -783,20 +841,6 @@ } /** - * Remove a column. - * - * @param col the column to remove - */ - public void removeColumn(Column<T, ?> col) { - int index = columns.indexOf(col); - if (index < 0) { - throw new IllegalArgumentException( - "The specified column is not part of this table."); - } - removeColumn(index); - } - - /** * Remove a style from the {@link TableColElement} at the specified index. * * @param index the column index @@ -886,7 +930,7 @@ } /** - * Render the header of footer. + * Render the header or footer. * * @param isFooter true if this is the footer table, false if the header table */ @@ -966,10 +1010,90 @@ return null; } + @SuppressWarnings("unchecked") + private <C> void fireEventToCell(Event event, String eventType, + TableCellElement tableCell, T value, int col, int row) { + Column<T, C> column = (Column<T, C>) columns.get(col); + Cell<C> cell = column.getCell(); + if (cellConsumesEventType(cell, eventType)) { + C cellValue = column.getValue(value); + Object key = providesKey == null ? value : providesKey.getKey(value); + boolean cellWasEditing = cell.isEditing(tableCell, cellValue, key); + column.onBrowserEvent( + tableCell, getPageStart() + row, value, event, providesKey); + cellIsEditing = cell.isEditing(tableCell, cellValue, key); + if (cellWasEditing && !cellIsEditing) { + view.resetFocus(); + } + } + } + private native int getClientHeight(Element element) /*-{ return element.clientHeight; }-*/; + private boolean handleKey(Event event) { + int keyCode = event.getKeyCode(); + int oldColumn = keyboardSelectedColumn; + int oldRow = keyboardSelectedRow; + int numColumns = columns.size(); + int numRows = getDataSize(); + switch (keyCode) { + case KeyCodes.KEY_UP: + if (keyboardSelectedRow > 0) { + --keyboardSelectedRow; + } + break; + case KeyCodes.KEY_DOWN: + if (keyboardSelectedRow < numRows - 1) { + ++keyboardSelectedRow; + } + break; + case KeyCodes.KEY_LEFT: + if (keyboardSelectedColumn == 0 && keyboardSelectedRow > 0) { + keyboardSelectedColumn = numColumns - 1; + --keyboardSelectedRow; + } else if (keyboardSelectedColumn > 0) { + --keyboardSelectedColumn; + } + break; + case KeyCodes.KEY_RIGHT: + if (keyboardSelectedColumn == numColumns - 1 + && keyboardSelectedRow < numRows - 1) { + keyboardSelectedColumn = 0; + ++keyboardSelectedRow; + } else if (keyboardSelectedColumn < numColumns - 1) { + ++keyboardSelectedColumn; + } + break; + } + + if (keyboardSelectedColumn != oldColumn || keyboardSelectedRow != oldRow) { + int pageStart = getPageStart(); + int pageSize = getPageSize(); + + // Remove old selection markers + TableRowElement row = getRowElement(oldRow - pageStart); + row.removeClassName(style.keyboardSelectedRow()); + TableCellElement td = row.getCells().getItem(oldColumn); + td.removeClassName(style.keyboardSelectedCell()); + td.removeAttribute("tabIndex"); + + // Move page start if needed + if (keyboardSelectedRow >= pageStart + pageSize) { + setPageStart(keyboardSelectedRow - pageSize + 1); + } else if (keyboardSelectedRow < pageStart) { + setPageStart(keyboardSelectedRow); + } + + // Add new selection markers and re-establish focus + view.resetFocus(); + return true; + } + + return false; + } + /** * Schedule a redraw for the end of the event loop. */
diff --git a/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java b/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java index aca6e84..87350a7 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java +++ b/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
@@ -222,6 +222,9 @@ loadChildState(values, 0, savedViews); } + public void resetFocus() { + } + public void setLoadingState(LoadingState state) { nodeView.updateImage(state == LoadingState.LOADING); showOrHide(nodeView.emptyMessageElem, state == LoadingState.EMPTY);
diff --git a/user/src/com/google/gwt/user/cellview/client/Column.java b/user/src/com/google/gwt/user/cellview/client/Column.java index d8ca946..6da6a4c 100644 --- a/user/src/com/google/gwt/user/cellview/client/Column.java +++ b/user/src/com/google/gwt/user/cellview/client/Column.java
@@ -70,7 +70,6 @@ /** * Render the object into the cell. - * * @param object the object to render * @param keyProvider the {@link ProvidesKey} for the object * @param sb the buffer to render into @@ -85,7 +84,7 @@ } /** - * Get the view keu for the object given the {@link ProvidesKey}. If the + * Get the view key for the object given the {@link ProvidesKey}. If the * {@link ProvidesKey} is null, the object is used as the key. * * @param object the row object
diff --git a/user/src/com/google/gwt/user/cellview/client/PagingListViewPresenter.java b/user/src/com/google/gwt/user/cellview/client/PagingListViewPresenter.java index 40e0681..2af1f04 100644 --- a/user/src/com/google/gwt/user/cellview/client/PagingListViewPresenter.java +++ b/user/src/com/google/gwt/user/cellview/client/PagingListViewPresenter.java
@@ -298,6 +298,11 @@ void replaceChildren(List<T> values, int start, String html); /** + * Re-establish focus on an element within the view if desired. + */ + void resetFocus(); + + /** * Set the current loading state of the data. * * @param state the loading state @@ -495,6 +500,9 @@ view.replaceChildren( boundedValues, boundedStart - pageStart, sb.toString()); } + + // Allow the view to reestablish focus after being re-rendered + view.resetFocus(); // Reset the pageStartChanged boolean. pageStartChangedSinceRender = false;
diff --git a/user/test/com/google/gwt/cell/client/CheckboxCellTest.java b/user/test/com/google/gwt/cell/client/CheckboxCellTest.java index 80efad7..073e7ce 100644 --- a/user/test/com/google/gwt/cell/client/CheckboxCellTest.java +++ b/user/test/com/google/gwt/cell/client/CheckboxCellTest.java
@@ -63,7 +63,7 @@ @Override protected String[] getConsumedEvents() { - return new String[]{"change"}; + return new String[]{"change", "keyup"}; } @Override
diff --git a/user/test/com/google/gwt/cell/client/EditTextCellTest.java b/user/test/com/google/gwt/cell/client/EditTextCellTest.java index ea94164..d4619fe 100644 --- a/user/test/com/google/gwt/cell/client/EditTextCellTest.java +++ b/user/test/com/google/gwt/cell/client/EditTextCellTest.java
@@ -47,7 +47,7 @@ * Cancel and switch to read only mode. */ public void testOnBrowserEventCancel() { - NativeEvent event = Document.get().createKeyDownEvent( + NativeEvent event = Document.get().createKeyUpEvent( false, false, false, false, KeyCodes.KEY_ESCAPE); ViewData viewData = new ViewData("originalValue"); viewData.setText("newValue"); @@ -63,7 +63,7 @@ * Cancel and switch to read only mode after committing once. */ public void testOnBrowserEventCancelSecondEdit() { - NativeEvent event = Document.get().createKeyDownEvent( + NativeEvent event = Document.get().createKeyUpEvent( false, false, false, false, KeyCodes.KEY_ESCAPE); ViewData viewData = new ViewData("originalValue"); viewData.setText("newValue"); @@ -87,7 +87,7 @@ * Commit and switch to read only mode. */ public void testOnBrowserEventCommit() { - NativeEvent event = Document.get().createKeyDownEvent( + NativeEvent event = Document.get().createKeyUpEvent( false, false, false, false, KeyCodes.KEY_ENTER); ViewData viewData = new ViewData("originalValue"); viewData.setText("newValue"); @@ -185,7 +185,7 @@ @Override protected String[] getConsumedEvents() { - return new String[]{"click", "keydown", "keyup", "blur"}; + return new String[]{"click", "keyup", "keydown", "blur"}; } @Override
diff --git a/user/test/com/google/gwt/user/cellview/client/PagingListViewPresenterTest.java b/user/test/com/google/gwt/user/cellview/client/PagingListViewPresenterTest.java index 7d4d182..79cb95e 100644 --- a/user/test/com/google/gwt/user/cellview/client/PagingListViewPresenterTest.java +++ b/user/test/com/google/gwt/user/cellview/client/PagingListViewPresenterTest.java
@@ -90,7 +90,6 @@ * @param <T> the data type */ private static class MockView<T> implements View<T> { - private int childCount; private boolean dependsOnSelection; private String lastHtml; @@ -174,6 +173,9 @@ public void setDependsOnSelection(boolean dependsOnSelection) { this.dependsOnSelection = dependsOnSelection; } + + public void resetFocus() { + } public void setLoadingState(LoadingState state) { this.loadingState = state;