Community
Participate
Working Groups
F3 build (20020612) In both the sync view and the compare editor, the Navigate / Next and Previous menu items (and accelerators) go to the next match but do not open the file. They should, especially considering the recent change to honour the Open Mode preference. Note that the toolbar buttons don't work at all, but that's a separate PR (bug 20105).
IMO the actions do the correct thing since we want to avoid loading large files from the server in this case. The rational behing this is that these actions reveal a new element and select it. So the user doesn't know which one will be the next file because the next file can be in an unexpanded folder. This would reintroduce the bug reported in bug 13730, but in a different usage scenario.
This is the same behaviour as before fixing bug 19972.
May be we should flush all downstream viewers when selecting a file (via prev/next or keyboard) when in double click mode. That would avoid the selection <-> compare mismatch.
I really think it should get the content anyway, despite the performance complaint. If you're using these buttons, you want to see the actual change. I think it will greatly hamper usability if the user has to hit the Next button, then double-click the file anyway. Why would they even bother using the Next button then?
Please consider for F4.
Agree should fix (+1)
Talked to Jeem, who found the performance problem especially painful when he was working with Philippe's repository in St. Nazaire from Ottawa. He also agrees Next and Previous should open, and that to do otherwise would break the flow.
*** Bug 20203 has been marked as a duplicate of this bug. ***
*** Bug 20282 has been marked as a duplicate of this bug. ***
Pressing next should open the diff and not only select it: - pressing the next action means show next diff (this is what we show in the tootip) - this is consistent with how show next is handled in the search results view.
*** Bug 20248 has been marked as a duplicate of this bug. ***
Important usability improvement. There were several complaints about the current behaviour. Fix is non trivial.
Since F3 compare uses open listeners instead of selection changed listeners to populate downstream viewers. This was necessary to fix bug 19972. The previous/next action are coded in a way that the programmatically change the selection to the next or previous difference. Since linking in comapre is now done using open listeners this doesn't populate the downstream viewers anymore. Fix is to generate an OpenEvent as well when the selection is change programmatically in these cases. This part is easy to fix. Only class effected is DiffTreeViewer. Since the initial selection doesn't populate the downstream viewers anymore (to avoid loading of files from the repository) first navigating to the next diff must be treated differently. Otherwise the first trigger of next would simply step to the next file, not to the next diff inside the first file. The reason is that the CompareNaviagtor tries to find the next diff by asking the installed panes(compare editor, structure compare 2, structure comapre 1, and comapre input) if the can provide a next diff. Since in the initial state only the comapre input pane is populated next jumps to the next diff in the campare input pane. Fix is to introduce a special interface IOpenable (not public) that works exactly the same way as INavigatable. If Next is pressed for the first time then the CompareNaviagtor opens the first file using this interface.
Created attachment 1443 [details] First patch for both problems
First review by Daniel Megert. Discovered a case where pressing next reveals the wrong element. This is the case when user opens the frist element by double clicking on it and then presses next. In this case the boolean fNextFristTime was still true so the element was reopened. Fixed by adding a new method mustOpen which checks if a downstream viewer is already populated when next is pressed the first time. private boolean mustOpen() { if (fPanes == null || fPanes.length == 0) return false; for (int i= 1; i < fPanes.length; i++) { CompareViewerSwitchingPane pane= fPanes[i]; if (pane != null && pane.getInput() != null) return false; } return true; }
Second review by Martin Aeschlimann
