Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Commit

Permalink
rhbz844820 - fix issue: focus of code mirror editor causes infinite l…
Browse files Browse the repository at this point in the history
…oop of selection change
  • Loading branch information
Patrick Huang committed Oct 17, 2012
1 parent 4496731 commit d0b54d3
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 57 deletions.
Expand Up @@ -55,6 +55,7 @@
import org.zanata.webtrans.shared.util.FindByTransUnitIdPredicate;
import com.allen_sauer.gwt.log.client.Log;
import com.google.common.base.Objects;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gwt.core.client.GWT;
Expand Down Expand Up @@ -158,14 +159,14 @@ private ToggleEditor getCurrentEditor()
return currentEditors.get(currentEditorIndex);
}

public void showEditors(final TransUnitId currentTransUnitId)
public void setSelected(final TransUnitId currentTransUnitId)
{
this.currentTransUnitId = currentTransUnitId;

editorTranslators.clearTranslatorList(currentEditors); // clear previous selection's translator list

display = findDisplayById(currentTransUnitId);
Log.info("enter show editor with id:" + currentTransUnitId + " version: " + display.getVerNum());
display = findDisplayById(currentTransUnitId).get();
Log.info("selecting id:" + currentTransUnitId + " version: " + display.getVerNum());

currentEditors = display.getEditors();

Expand All @@ -191,17 +192,9 @@ public void showEditors(final TransUnitId currentTransUnitId)
}
}

private TargetContentsDisplay findDisplayById(TransUnitId currentTransUnitId)
private Optional<TargetContentsDisplay> findDisplayById(TransUnitId currentTransUnitId)
{
try
{
return Iterables.find(displayList, new FindByTransUnitIdPredicate(currentTransUnitId));
}
catch (NoSuchElementException e)
{
Log.error("cannot find display by id:" + currentTransUnitId + ". Page has changed?! returning null");
return null;
}
return Iterables.tryFind(displayList, new FindByTransUnitIdPredicate(currentTransUnitId));
}

private void normaliseCurrentEditorIndex()
Expand All @@ -218,7 +211,7 @@ private void normaliseCurrentEditorIndex()
}

@Override
public void onTransUnitEdit(final TransUnitEditEvent event)
public void onTransUnitEdit(TransUnitEditEvent event)
{
if (event.getSelectedTransUnit() != null)
{
Expand Down Expand Up @@ -309,11 +302,6 @@ public TransUnitId getCurrentTransUnitIdOrNull()
return currentTransUnitId;
}

public TransUnit getCachedValue()
{
return hasSelectedRow() ? display.getCachedValue() : null;
}

@Override
public boolean isDisplayButtons()
{
Expand All @@ -334,10 +322,10 @@ public void showHistory(TransUnitId transUnitId)
}

@Override
public void onFocus(TransUnitId id, int editorIndex)
public void onEditorClicked(TransUnitId id, int editorIndex)
{
ensureRowSelection(id);
currentEditorIndex = editorIndex;
ensureRowSelection(id);
}

@Override
Expand All @@ -359,16 +347,16 @@ private void ensureRowSelection(TransUnitId transUnitId)
{
if (!equal(currentTransUnitId, transUnitId))
{
//user click on buttons that is not on current selected row
//user click on editor area that is not on current selected row
eventBus.fireEvent(new TableRowSelectedEvent(transUnitId));
}
}

@Override
public void copySource(ToggleEditor editor, TransUnitId id)
{
ensureRowSelection(id);
currentEditorIndex = editor.getIndex();
ensureRowSelection(id);
editor.setTextAndValidate(sourceContentsPresenter.getSelectedSource());
editor.setFocus();

Expand Down Expand Up @@ -497,9 +485,10 @@ public void highlightSearch(String message)
*/
public void updateRow(TransUnit updatedTransUnit)
{
TargetContentsDisplay contentsDisplay = findDisplayById(updatedTransUnit.getId());
if (contentsDisplay != null)
Optional<TargetContentsDisplay> contentsDisplayOptional = findDisplayById(updatedTransUnit.getId());
if (contentsDisplayOptional.isPresent())
{
TargetContentsDisplay contentsDisplay = contentsDisplayOptional.get();
contentsDisplay.setValue(updatedTransUnit);
contentsDisplay.setState(TargetContentsDisplay.EditingState.SAVED);
}
Expand All @@ -512,9 +501,10 @@ public void updateRow(TransUnit updatedTransUnit)
*/
public void confirmSaved(TransUnit updatedTU)
{
TargetContentsDisplay contentsDisplay = findDisplayById(updatedTU.getId());
if (contentsDisplay != null)
Optional<TargetContentsDisplay> contentsDisplayOptional = findDisplayById(updatedTU.getId());
if (contentsDisplayOptional.isPresent())
{
TargetContentsDisplay contentsDisplay = contentsDisplayOptional.get();
contentsDisplay.updateCachedTargetsAndVersion(updatedTU.getTargets(), updatedTU.getVerNum(), updatedTU.getStatus());
setEditingState(updatedTU.getId(), TargetContentsDisplay.EditingState.SAVED);
}
Expand Down Expand Up @@ -571,17 +561,18 @@ else if (!userWorkspaceContext.hasReadOnlyAccess())
@Override
public void setEditingState(TransUnitId transUnitId, TargetContentsDisplay.EditingState editingState)
{
TargetContentsDisplay display = findDisplayById(transUnitId);
if (display != null && editingState != display.getEditingState())
Optional<TargetContentsDisplay> displayOptional = findDisplayById(transUnitId);
TargetContentsDisplay contentsDisplay = displayOptional.orNull();
if (contentsDisplay != null && editingState != contentsDisplay.getEditingState())
{
if (editingState != TargetContentsDisplay.EditingState.SAVED)
{
display.setState(editingState);
contentsDisplay.setState(editingState);
}
else if (Objects.equal(display.getCachedTargets(), display.getNewTargets()))
else if (Objects.equal(contentsDisplay.getCachedTargets(), contentsDisplay.getNewTargets()))
{
// we set editing state to SAVED only if cached targets and in editor targets are equal
display.setState(TargetContentsDisplay.EditingState.SAVED);
contentsDisplay.setState(TargetContentsDisplay.EditingState.SAVED);
}
}
}
Expand Down
Expand Up @@ -151,7 +151,7 @@ public void onTransUnitSelected(TransUnitSelectionEvent event)
selectedId = selection.getId();
Log.debug("selected id: " + selectedId);
sourceContentsPresenter.setSelectedSource(selectedId);
targetContentsPresenter.showEditors(selectedId);
targetContentsPresenter.setSelected(selectedId);
translatorService.transUnitSelected(selection);
}

Expand Down
14 changes: 4 additions & 10 deletions zanata-war/src/main/java/org/zanata/webtrans/client/ui/Editor.java
Expand Up @@ -10,7 +10,6 @@
import com.google.gwt.core.client.GWT;
import com.google.gwt.event.dom.client.BlurEvent;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.FocusEvent;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.resources.client.CssResource;
import com.google.gwt.uibinder.client.UiBinder;
Expand Down Expand Up @@ -50,9 +49,6 @@ interface Styles extends CssResource

private static EditorUiBinder uiBinder = GWT.create(EditorUiBinder.class);

private static final int TYPING_TIMER_INTERVAL = 500; // ms
private static final int TYPING_TIMER_RECURRENT_VALIDATION_PERIOD = 5; // intervals

private final int index;
private final TransUnitId id;

Expand Down Expand Up @@ -117,12 +113,11 @@ public void onValueChange(ValueChangeEvent<String> event)
listener.setEditingState(id, editingState);
}

@UiHandler("textArea")
public void onTextAreaFocus(FocusEvent event)
@UiHandler("rootContainer")
public void onEditorClick(ClickEvent event)
{
listener.onFocus(id, index);
listener.onEditorClicked(id, index);
fireValidationEvent();
isFocused = true;
}

@UiHandler("textArea")
Expand Down Expand Up @@ -186,6 +181,7 @@ public String getText()
@Override
public void setFocus()
{
isFocused = true;
textArea.setFocus(true);
}

Expand Down Expand Up @@ -230,12 +226,10 @@ public void updateValidationWarning(List<String> errors)
if (!errors.isEmpty())
{
targetWrapper.addStyleName(style.hasValidationError());
// addStyleName(style.hasValidationError());
}
else
{
targetWrapper.removeStyleName(style.hasValidationError());
// removeStyleName(style.hasValidationError());
}
}

Expand Down
Expand Up @@ -90,7 +90,7 @@ interface Listener

void showHistory(TransUnitId transUnitId);

void onFocus(TransUnitId id, int editorIndex);
void onEditorClicked(TransUnitId id, int editorIndex);

boolean isUsingCodeMirror();

Expand Down
Expand Up @@ -47,12 +47,10 @@
import org.zanata.webtrans.client.events.TransUnitSaveEvent;
import org.zanata.webtrans.client.events.UserConfigChangeEvent;
import org.zanata.webtrans.client.events.WorkspaceContextUpdateEvent;
import org.zanata.webtrans.client.resources.NavigationMessages;
import org.zanata.webtrans.client.resources.TableEditorMessages;
import org.zanata.webtrans.client.ui.ToggleEditor;
import org.zanata.webtrans.client.ui.UndoLink;
import org.zanata.webtrans.client.view.TargetContentsDisplay;
import org.zanata.webtrans.shared.auth.Identity;
import org.zanata.webtrans.shared.model.TransUnit;
import org.zanata.webtrans.shared.model.TransUnitId;
import org.zanata.webtrans.shared.model.UserWorkspaceContext;
Expand Down Expand Up @@ -98,13 +96,10 @@ public class TargetContentsPresenterTest
@Mock private EventBus eventBus;
@Mock private TableEditorMessages tableEditorMessages;
@Mock private SourceContentsPresenter sourceContentPresenter;
@Mock private NavigationMessages navMessages;
private UserWorkspaceContext userWorkspaceContext;
@Mock private TargetContentsDisplay display;
@Mock
private ToggleEditor editor, editor2;
@Mock
private Identity identity;
private TransUnit selectedTU;

// all event extends GwtEvent therefore captor will capture them all
Expand Down Expand Up @@ -188,7 +183,7 @@ public void canCopySource()
selectedTU = currentPageRows.get(0);
when(display.getId()).thenReturn(selectedTU.getId());
when(display.getEditors()).thenReturn(Lists.newArrayList(editor));
presenter.showEditors(selectedTU.getId());
presenter.setSelected(selectedTU.getId());
when(sourceContentPresenter.getSelectedSource()).thenReturn("source");

presenter.copySource(editor, selectedTU.getId());
Expand Down Expand Up @@ -271,7 +266,7 @@ public void testOnInsertString()
when(display.getEditors()).thenReturn(Lists.newArrayList(editor));
when(tableEditorMessages.notifyCopied()).thenReturn("copied");
when(sourceContentPresenter.getSelectedSource()).thenReturn("source content");
presenter.showEditors(selectedTU.getId());
presenter.setSelected(selectedTU.getId());

// When:
presenter.onInsertString(new InsertStringInEditorEvent("", "suggestion"));
Expand All @@ -294,7 +289,7 @@ public void testOnTransMemoryCopy()
selectedTU = currentPageRows.get(0);
when(display.getId()).thenReturn(selectedTU.getId());
when(display.getEditors()).thenReturn(Lists.newArrayList(editor));
presenter.showEditors(selectedTU.getId());
presenter.setSelected(selectedTU.getId());

presenter.onDataCopy(new CopyDataToEditorEvent(Arrays.asList("target")));

Expand All @@ -311,7 +306,7 @@ public void willMoveToNextEntryIfItIsPluralAndNotAtLastEntry()
selectedTU = currentPageRows.get(0);
when(display.getId()).thenReturn(selectedTU.getId());
when(display.getEditors()).thenReturn(Lists.newArrayList(editor, editor2));
presenter.showEditors(selectedTU.getId());
presenter.setSelected(selectedTU.getId());

// When:
presenter.saveAsApprovedAndMoveNext(selectedTU.getId());
Expand All @@ -329,7 +324,7 @@ public void willSaveAndMoveToNextRow()
when(display.getCachedTargets()).thenReturn(CACHED_TARGETS);
when(display.getId()).thenReturn(selectedTU.getId());
when(display.getEditors()).thenReturn(Lists.newArrayList(editor));
presenter.showEditors(selectedTU.getId());
presenter.setSelected(selectedTU.getId());

// When:
presenter.saveAsApprovedAndMoveNext(selectedTU.getId());
Expand Down Expand Up @@ -438,7 +433,7 @@ public void testOnFocus()
TransUnitId oldSelection = currentPageRows.get(1).getId();
presenter.setStatesForTesting(oldSelection, 0, display, null);

presenter.onFocus(selectedTU.getId(), 1);
presenter.onEditorClicked(selectedTU.getId(), 1);

verify(eventBus).fireEvent(eventCaptor.capture());
TableRowSelectedEvent tableRowSelectedEvent = TestFixture.extractFromEvents(eventCaptor.getAllValues(), TableRowSelectedEvent.class);
Expand Down Expand Up @@ -740,7 +735,7 @@ public void testShowEditorsInReadOnlyMode()
when(display.getEditors()).thenReturn(currentEditors);

// When:
presenter.showEditors(selectedTU.getId());
presenter.setSelected(selectedTU.getId());

// Then:
verify(editorTranslators).clearTranslatorList(previousEditors);
Expand Down
Expand Up @@ -109,7 +109,7 @@ public void onTransUnitSelected()

// Then:
verify(sourceContentsPresenter).setSelectedSource(selection.getId());
verify(targetContentsPresenter).showEditors(selection.getId());
verify(targetContentsPresenter).setSelected(selection.getId());
verify(translatorService).transUnitSelected(selection);
}

Expand Down

0 comments on commit d0b54d3

Please sign in to comment.