[BUG] Fix: AutoScrollToFirstChange() was not working reliably for Side-By-Side Diff #1952
Conversation
Both of the SingleSideTextDiffPresenter instances need to be scrolled/synced for this to work reliably. (To see where it failed, open a Side-By-Side Diff where the first change-block would be out-of-view, then toggle `Show All Lines` - when enabling the latter, the first change-block would NOT be correctly scrolled-to.)
* Added private helper method GotoChange(), to remove redundant code. * Made `SyncScrollOffset()` virtual, so the `Goto<...>Change()` methods could be removed. * In `DiffContext.CheckSettings()`, setting the Context is now made on a single (less redundant) line, just like in the `DiffContext.UseSideBySide` setter.
goran-w
force-pushed
the
fix_jump_to_first_change_in_two_sided_diff
branch
from
396f15b to
ebc8d27
Compare
goran-w
changed the title
|
|
Signed-off-by: leo <longshuang@msn.cn>
|
I've pushed my fix for this issue |
Signed-off-by: leo <longshuang@msn.cn>
Great - looks good! Seems like the A possible future improvement could be to jump to the current change-block (instead of back to the first one) when toggling It's all about not losing context, when toggling these options - which I often do specifically to get another view of the very same change-block I'm looking at... |
#1952) Signed-off-by: leo <longshuang@msn.cn>
|
Excellent work on keeping the current change-block when toggling Could we also have this when toggling For that, it would probably be (more or less) sufficient to make the following changes :
|
…ew (#1952) Signed-off-by: leo <longshuang@msn.cn>
|
Marvelous, current change-block is now kept also when toggling |
2 participants
Both of the
SingleSideTextDiffPresenterinstances need to be scrolled/synced inAutoScrollToFirstChange(), for the auto-scrolling to work reliably. (To see where it failed, open aSide-By-Side Diffwhere the first change-block would be out-of-view, then toggleShow All Lines- when enabling the latter, the first change-block would NOT be correctly scrolled-to.)As a bonus, a bit of cleanup and simplification was also done in related code:
SyncScrollOffset()virtual, so theGoto<...>Change()methods could be removed.DiffContext.CheckSettings(), setting theContextis now made on a single (less redundant) line, just like in theDiffContext.UseSideBySidesetter.