From 350cf09843704ae8f8a86983556afef5e33b76b9 Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 10 May 2012 17:22:56 +1000 Subject: [PATCH] add UI elements for undoing single translation replacement --- .../presenter/SearchResultsPresenter.java | 274 +++++++++++++++--- .../client/view/SearchResultsView.java | 123 ++++++-- 2 files changed, 318 insertions(+), 79 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java index 8f4d137be5..d3dc7bd025 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java @@ -22,9 +22,12 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import net.customware.gwt.presenter.client.EventBus; import net.customware.gwt.presenter.client.widget.WidgetDisplay; @@ -99,25 +102,28 @@ public interface Display extends WidgetDisplay HasClickHandlers addDocumentLabel(String docName); - HasData addTUTable(Delegate replaceDelegate, SelectionModel selectionModel, ValueChangeHandler selectAllHandler); + HasData addTUTable(Delegate replaceDelegate, Delegate undoDelegate, SelectionModel selectionModel, ValueChangeHandler selectAllHandler); } private final CachingDispatchAsync dispatcher; private final History history; private AsyncCallback projectSearchCallback; - private Delegate replaceButtonDelegate; + private Delegate replaceButtonDelegate; + private Delegate undoButtonDelegate; + private Comparator tuInfoComparator; + /** * Model objects for tables in display. Changes to these are reflected in the * view. */ - private Map> documentDataProviders; + private Map> documentDataProviders; /** * Selection model objects for tables in display. Used to determine which * transunits are selected */ - private Map> documentSelectionModels; + private Map> documentSelectionModels; /** * most recent history state that was responded to @@ -137,8 +143,11 @@ protected void onBind() { projectSearchCallback = buildProjectSearchCallback(); replaceButtonDelegate = buildReplaceButtonDelegate(); - documentDataProviders = new HashMap>(); - documentSelectionModels = new HashMap>(); + undoButtonDelegate = buildUndoButtonDelegate(); + documentDataProviders = new HashMap>(); + documentSelectionModels = new HashMap>(); + tuInfoComparator = buildTransUnitReplaceInfoComparator(); + registerHandler(display.getFilterTextBox().addValueChangeHandler(new ValueChangeHandler() { @@ -208,9 +217,12 @@ public void onChange(ChangeEvent event) public void onClick(ClickEvent event) { List selected = new ArrayList(); - for (MultiSelectionModel sm : documentSelectionModels.values()) + for (MultiSelectionModel sm : documentSelectionModels.values()) { - selected.addAll(sm.getSelectedSet()); + for (TransUnitReplaceInfo info : sm.getSelectedSet()) + { + selected.add(info.getTransUnit()); + } } if (selected.isEmpty()) @@ -271,43 +283,43 @@ public void onValueChange(ValueChangeEvent event) public void onTransUnitUpdated(final TransUnitUpdatedEvent event) { TransUnitUpdateInfo updateInfo = event.getUpdateInfo(); - ListDataProvider dataProvider = documentDataProviders.get(updateInfo.getDocumentId().getId()); - if (dataProvider == null) + TransUnitReplaceInfo replaceInfo = getReplaceInfoForUpdatedTU(updateInfo); + if (replaceInfo == null) { - Log.debug("document '" + updateInfo.getDocumentId().getId() + "' not found for TU update, id: " + updateInfo.getTransUnit().getId().getId()); + Log.debug("no matching TU in document for TU update, id: " + updateInfo.getTransUnit().getId().getId()); return; } + Log.debug("found matching TU for TU update, id: " + updateInfo.getTransUnit().getId().getId()); - List transUnits = dataProvider.getList(); - //TransUnit does not appear to have .equals(o), so list items are manually compared - for (int i = 0; i < transUnits.size(); i++) + MultiSelectionModel selectionModel = documentSelectionModels.get(updateInfo.getDocumentId().getId()); + if (selectionModel == null) { - if (transUnits.get(i).getId().getId() == updateInfo.getTransUnit().getId().getId()) - { - Log.debug("found matching TU for TU update, id: " + updateInfo.getTransUnit().getId().getId()); - // must de-select before setting to prevent this TU being - // 'stuck' selected for the life of the selection model - // Leaving de-selected as this is currently the desired behaviour - MultiSelectionModel selectionModel = documentSelectionModels.get(updateInfo.getDocumentId().getId()); -// boolean isSelected = selectionModel.isSelected(transUnits.get(i)); //get selected state - if (selectionModel == null) - { - Log.error("missing selection model for document, id: " + updateInfo.getDocumentId().getId()); - } - else - { - selectionModel.setSelected(transUnits.get(i), false); - } + Log.error("missing selection model for document, id: " + updateInfo.getDocumentId().getId()); + } + else + { + // no need to do this as replaceInfo only has properties changed + // safe to remove if desired behaviour is keeping selected + selectionModel.setSelected(replaceInfo, false); + } - transUnits.set(i, updateInfo.getTransUnit()); -// selectionModel.setSelected(transUnits.get(i), isSelected); //return to previous selection state + if (replaceInfo.isUndoable() && replaceInfo.getTransUnit().getVerNum() != updateInfo.getTransUnit().getVerNum()) + { + // can't undo after new update + replaceInfo.setUndoable(false); + replaceInfo.setReplaceable(true); + replaceInfo.setReplacing(false); + replaceInfo.setReplaceInfo(null); + } + replaceInfo.setTransUnit(updateInfo.getTransUnit()); - return; - } + // force table refresh as property changes are not detected + ListDataProvider dataProvider = documentDataProviders.get(updateInfo.getDocumentId().getId()); + if (dataProvider != null) + { + dataProvider.refresh(); } - Log.debug("no matching TU in document for TU update, id: " + updateInfo.getTransUnit().getId().getId()); } - })); } @@ -366,11 +378,11 @@ public void onClick(ClickEvent event) }); - final MultiSelectionModel selectionModel = new MultiSelectionModel(); - final ListDataProvider dataProvider = new ListDataProvider(); + final MultiSelectionModel selectionModel = new MultiSelectionModel(); + final ListDataProvider dataProvider = new ListDataProvider(); // TODO set selected value for header based on selection of rows? - HasData table = display.addTUTable(replaceButtonDelegate, selectionModel, new ValueChangeHandler() + HasData table = display.addTUTable(replaceButtonDelegate, undoButtonDelegate, selectionModel, new ValueChangeHandler() { @Override @@ -378,9 +390,9 @@ public void onValueChange(ValueChangeEvent event) { if (event.getValue()) { - for (TransUnit tu : dataProvider.getList()) + for (TransUnitReplaceInfo info : dataProvider.getList()) { - selectionModel.setSelected(tu, true); + selectionModel.setSelected(info, true); } } else @@ -392,9 +404,12 @@ public void onValueChange(ValueChangeEvent event) }); dataProvider.addDataDisplay(table); - List data = dataProvider.getList(); - data.addAll(result.getUnits(docId)); - Collections.sort(data, TransUnit.getRowIndexComparator()); + List data = dataProvider.getList(); + for (TransUnit tu : result.getUnits(docId)) + { + data.add(new TransUnitReplaceInfo(tu)); + } + Collections.sort(data, tuInfoComparator); documentDataProviders.put(docId, dataProvider); documentSelectionModels.put(docId, selectionModel); } @@ -403,14 +418,28 @@ public void onValueChange(ValueChangeEvent event) }; } - private Delegate buildReplaceButtonDelegate() + private Delegate buildReplaceButtonDelegate() + { + return new Delegate() { + + @Override + public void execute(TransUnitReplaceInfo tu) + { + fireReplaceTextEvent(Collections.singletonList(tu.getTransUnit())); + } + }; + } + + private Delegate buildUndoButtonDelegate() { - return new Delegate() { + return new Delegate() { @Override - public void execute(TransUnit tu) + public void execute(TransUnitReplaceInfo info) { - fireReplaceTextEvent(Collections.singletonList(tu)); + eventBus.fireEvent(new NotificationEvent(Severity.Error, "Undo not implemented")); + // FIXME fire this when it exists +// fireUndoUpdateEvent(Collections.singletonList(tu)); } }; } @@ -420,6 +449,7 @@ public void execute(TransUnit tu) */ private void fireReplaceTextEvent(List toReplace) { + // TODO set info to replacing... before calling this event String searchText = currentHistoryState.getProjectSearchText(); String replacement = currentHistoryState.getProjectSearchReplacement(); boolean caseSensitive = currentHistoryState.getProjectSearchCaseSensitive(); @@ -433,15 +463,163 @@ public void onFailure(Throwable e) Log.error("[SearchResultsPresenter] Replace text failure " + e, e); // TODO use localised string eventBus.fireEvent(new NotificationEvent(Severity.Error, "Replace text failed")); + // TODO consider whether possible/desired to change TU state from 'replacing' } @Override public void onSuccess(UpdateTransUnitResult result) { + Set updatedDocs = new HashSet(); + for (TransUnitUpdateInfo updateInfo : result.getUpdateInfoList()) + { + if (updateInfo.isSuccess()) + { + TransUnitReplaceInfo replaceInfo = getReplaceInfoForUpdatedTU(updateInfo); + if (replaceInfo != null) + { + replaceInfo.setReplaceInfo(updateInfo); + replaceInfo.setUndoable(true); + replaceInfo.setReplaceable(false); + replaceInfo.setReplacing(false); + //this should be done when the TU update event comes in anyway + //may want to remove this + replaceInfo.setTransUnit(updateInfo.getTransUnit()); + } + updatedDocs.add(updateInfo.getDocumentId().getId()); + } + // individual failure behaviour not yet defined + } + + // force table refresh as property changes are not detected + for (Long docId : updatedDocs) + { + ListDataProvider dataProvider = documentDataProviders.get(docId); + if (dataProvider != null) + { + dataProvider.refresh(); + } + } + // TODO use localised string eventBus.fireEvent(new NotificationEvent(Severity.Info, "Successfully replaced text")); } }); } + public class TransUnitReplaceInfo + { + private TransUnit tu; + private TransUnitUpdateInfo replaceInfo = null; + private boolean replaceable; + private boolean replacing; + private boolean undoable; + + public TransUnitReplaceInfo(TransUnit tu) + { + this.tu = tu; + replaceInfo = null; + replaceable = true; + replacing = false; + undoable = false; + } + + public TransUnit getTransUnit() + { + return tu; + } + + public void setTransUnit(TransUnit tu) + { + this.tu = tu; + } + + public TransUnitUpdateInfo getReplaceInfo() + { + return replaceInfo; + } + + public void setReplaceInfo(TransUnitUpdateInfo replaceInfo) + { + this.replaceInfo = replaceInfo; + } + + public boolean isReplaceable() + { + return replaceable; + } + + public void setReplaceable(boolean canReplace) + { + this.replaceable = canReplace; + } + + public boolean isReplacing() + { + return replacing; + } + + public void setReplacing(boolean isReplacing) + { + this.replacing = isReplacing; + } + + public boolean isUndoable() + { + return undoable; + } + + public void setUndoable(boolean undoable) + { + this.undoable = undoable; + } + + } + + private Comparator buildTransUnitReplaceInfoComparator() + { + return new Comparator() + { + + @Override + public int compare(TransUnitReplaceInfo o1, TransUnitReplaceInfo o2) + { + if (o1 == o2) + { + return 0; + } + if (o1 != null) + { + return (o2 != null ? Integer.valueOf(o1.getTransUnit().getRowIndex()).compareTo(o2.getTransUnit().getRowIndex()) : 1); + } + return -1; + } + }; + } + + /** + * @param updateInfo + * @param replaceInfo + * @return the replace info for the updated {@link TransUnit}, or null if the + * current search results do not contain the TransUnit + */ + private TransUnitReplaceInfo getReplaceInfoForUpdatedTU(TransUnitUpdateInfo updateInfo) + { + ListDataProvider dataProvider = documentDataProviders.get(updateInfo.getDocumentId().getId()); + if (dataProvider == null) + { + Log.debug("document '" + updateInfo.getDocumentId().getId() + "' not found for TU update, id: " + updateInfo.getTransUnit().getId().getId()); + return null; + } + + List replaceInfoList = dataProvider.getList(); + //TransUnit does not appear to have .equals(o), so list items are manually compared + for (TransUnitReplaceInfo info : replaceInfoList) + { + if (info.getTransUnit().getId().getId() == updateInfo.getTransUnit().getId().getId()) + { + return info; + } + } + return null; + } } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java index 763bf08f89..1a3eed8c7d 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java @@ -25,11 +25,11 @@ import org.zanata.common.ContentState; import org.zanata.webtrans.client.presenter.SearchResultsPresenter; +import org.zanata.webtrans.client.presenter.SearchResultsPresenter.TransUnitReplaceInfo; import org.zanata.webtrans.client.resources.Resources; import org.zanata.webtrans.client.resources.UiMessages; import org.zanata.webtrans.client.ui.ClearableTextBox; import org.zanata.webtrans.client.ui.HighlightingLabel; -import org.zanata.webtrans.shared.model.TransUnit; import com.google.common.base.Predicate; import com.google.common.base.Strings; @@ -39,6 +39,7 @@ import com.google.gwt.cell.client.ActionCell.Delegate; import com.google.gwt.cell.client.Cell.Context; import com.google.gwt.cell.client.CheckboxCell; +import com.google.gwt.cell.client.ValueUpdater; import com.google.gwt.core.client.GWT; import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.NativeEvent; @@ -50,7 +51,9 @@ import com.google.gwt.event.shared.GwtEvent; import com.google.gwt.event.shared.HandlerManager; import com.google.gwt.event.shared.HandlerRegistration; +import com.google.gwt.safehtml.shared.SafeHtml; import com.google.gwt.safehtml.shared.SafeHtmlBuilder; +import com.google.gwt.safehtml.shared.SafeHtmlUtils; import com.google.gwt.uibinder.client.UiBinder; import com.google.gwt.uibinder.client.UiField; import com.google.gwt.user.cellview.client.CellTable; @@ -176,16 +179,17 @@ public HasClickHandlers addDocumentLabel(String docName) } @Override - public HasData addTUTable(Delegate replaceDelegate, - SelectionModel selectionModel, + public HasData addTUTable(Delegate replaceDelegate, + Delegate undoDelegate, + SelectionModel selectionModel, ValueChangeHandler selectAllHandler) { - CellTable table = buildTable(replaceDelegate, selectionModel, selectAllHandler); + CellTable table = buildTable(replaceDelegate, undoDelegate, selectionModel, selectAllHandler); searchResultsPanel.add(table); return table; } - private CellTable buildTable(Delegate replaceDelegate, SelectionModel selectionModel, + private CellTable buildTable(Delegate replaceDelegate, Delegate undoDelegate, SelectionModel selectionModel, ValueChangeHandler selectAllHandler) { // create columns @@ -193,16 +197,16 @@ private CellTable buildTable(Delegate replaceDelegate, Sel CheckboxHeader checkboxColumnHeader = new CheckboxHeader(); checkboxColumnHeader.addValueChangeHandler(selectAllHandler); - TextColumn rowIndexColumn = new TextColumn() + TextColumn rowIndexColumn = new TextColumn() { @Override - public String getValue(TransUnit tu) + public String getValue(TransUnitReplaceInfo tu) { - return Integer.toString(tu.getRowIndex()); + return Integer.toString(tu.getTransUnit().getRowIndex()); } }; - Column> sourceColumn = new Column>(new AbstractCell>() + Column> sourceColumn = new Column>(new AbstractCell>() { @Override public void render(Context context, List contents, SafeHtmlBuilder sb) @@ -217,13 +221,13 @@ public void render(Context context, List contents, SafeHtmlBuilder sb) }) { @Override - public List getValue(TransUnit transUnit) + public List getValue(TransUnitReplaceInfo info) { - return transUnit.getSources(); + return info.getTransUnit().getSources(); } }; - Column> targetColumn = new Column>(new AbstractCell>() + Column> targetColumn = new Column>(new AbstractCell>() { @Override public void render(Context context, List contents, SafeHtmlBuilder sb) @@ -243,20 +247,20 @@ public void render(Context context, List contents, SafeHtmlBuilder sb) { @Override - public List getValue(TransUnit tu) + public List getValue(TransUnitReplaceInfo info) { - return tu.getTargets(); + return info.getTransUnit().getTargets(); } @Override - public String getCellStyleNames(Context context, TransUnit tu) + public String getCellStyleNames(Context context, TransUnitReplaceInfo info) { - String styleNames = Strings.nullToEmpty(super.getCellStyleNames(context, tu)); - if (tu.getStatus() == ContentState.Approved) + String styleNames = Strings.nullToEmpty(super.getCellStyleNames(context, info)); + if (info.getTransUnit().getStatus() == ContentState.Approved) { styleNames += " ApprovedStateDecoration"; } - else if (tu.getStatus() == ContentState.NeedReview) + else if (info.getTransUnit().getStatus() == ContentState.NeedReview) { styleNames += " FuzzyStateDecoration"; } @@ -264,10 +268,10 @@ else if (tu.getStatus() == ContentState.NeedReview) } }; - ReplaceColumn replaceButtonColumn = new ReplaceColumn(replaceDelegate); + ReplaceColumn replaceButtonColumn = new ReplaceColumn(replaceDelegate, undoDelegate); - CellTable table = new CellTable(); - table.setSelectionModel(selectionModel, DefaultSelectionEventManager. createCheckboxManager()); + CellTable table = new CellTable(); + table.setSelectionModel(selectionModel, DefaultSelectionEventManager. createCheckboxManager()); table.setWidth("100%", true); // TODO use localisable headings (should already exist somewhere) @@ -306,21 +310,21 @@ public boolean apply(String input) }); } - private class CheckColumn extends Column + private class CheckColumn extends Column { - private SelectionModel selectionModel; + private SelectionModel selectionModel; - public CheckColumn(SelectionModel selectionModel) + public CheckColumn(SelectionModel selectionModel) { super(new CheckboxCell(true, false)); this.selectionModel = selectionModel; } @Override - public Boolean getValue(TransUnit tu) + public Boolean getValue(TransUnitReplaceInfo info) { - return selectionModel.isSelected(tu); + return selectionModel.isSelected(info); } } @@ -394,21 +398,78 @@ private HandlerManager ensureHandlerManager() } } - private class ReplaceColumn extends Column + private class ReplaceColumn extends Column { - public ReplaceColumn(Delegate replaceDelegate) + public ReplaceColumn(Delegate replaceDelegate, Delegate undoDelegate) { - super(new ActionCell("Replace", replaceDelegate)); + super(new UndoableTransUnitActionCell("Replace", replaceDelegate, "Undo", undoDelegate)); } @Override - public TransUnit getValue(TransUnit tu) + public TransUnitReplaceInfo getValue(TransUnitReplaceInfo info) { - return tu; + return info; } } + private class UndoableTransUnitActionCell extends ActionCell + { + private Delegate undoDelegate; + private final SafeHtml undoHtml; + + public UndoableTransUnitActionCell(SafeHtml actionLabel, Delegate actionDelegate, SafeHtml undoLabel, Delegate undoDelegate) { + super(actionLabel, actionDelegate); + this.undoDelegate = undoDelegate; + // TODO make this a 'replaced' message with undo button or link + this.undoHtml = new SafeHtmlBuilder().appendHtmlConstant( + "").toSafeHtml(); + } + + public UndoableTransUnitActionCell(String actionLabel, Delegate actionDelegate, String undoLabel, Delegate undoDelegate) + { + this(SafeHtmlUtils.fromString(actionLabel), actionDelegate, SafeHtmlUtils.fromString(undoLabel), undoDelegate); + } + + @Override + public void render(com.google.gwt.cell.client.Cell.Context context, TransUnitReplaceInfo value, SafeHtmlBuilder sb) + { + + if (value.isReplaceable()) + { + // render default button + super.render(context, value, sb); + } + else if (value.isUndoable()) + { + // render undo button + sb.append(undoHtml); + } + else + { + //assume processing + // TODO set to show "replacing..." or "processing..." + } + + } + + @Override + protected void onEnterKeyDown(Context context, Element parent, TransUnitReplaceInfo value, NativeEvent event, ValueUpdater valueUpdater) { + // FIXME see above + if (value.isReplaceable()) + { + super.onEnterKeyDown(context, parent, value, event, valueUpdater); + } + else if (value.isUndoable()) + { + undoDelegate.execute(value); + } + // else ignore (is processing) + }; + } + + @Override public HasChangeHandlers getSearchFieldSelector() {