Show FormDiff modeless. #4807
Show FormDiff modeless. #4807
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4807 +/- ##
==========================================
+ Coverage 30.49% 30.66% +0.17%
==========================================
Files 519 520 +1
Lines 41880 42023 +143
Branches 5898 5908 +10
==========================================
+ Hits 12770 12887 +117
- Misses 28586 28610 +24
- Partials 524 526 +2
Continue to review full report at Codecov.
|
GitUI/UserControls/RevisionGrid.cs
Outdated
| { | ||
| diffForm.ShowDialog(this); | ||
| } | ||
| var diffForm = new FormDiff(UICommands, this, baseCommit, headCommit.Guid, |
There was a problem hiding this comment.
I think we could introduce a method, no!?!
|
Could you please provide a bit more info on the nature of the change and the benefits it brings? |
|
Can't speak for @jbialobr but I am in favour of anything that converts modal forms to non-modal ones. Workflows with GE involve multiple windows and when you can't click between them it's really frustrating. I understand there are cases with technical reasons behind them, but they're just limitations of the design rather than some fundamental problem, so we should work towards removing modality where possible. |
|
As @drewnoakes said, there are cases when you need some data from other forms (mostly main form) and when the FormDiff is modal you can't go to the main form to see the data. Scenario: you are in the middle of reviewing in FormDiff and want to read the commit message of the commit that introduced a change you are looking at. I changed it to WIP because FormDiff depends on revisionGrid. I have to remove that dependency to make it work properly. |
OpenWithDifftool does not need RevisionGrid. It needs IWin32Window and GitModule. It fits better as a member of UICommands.
FormDiff receives only immutable dependencies and can be shown modeless (is invariant to repo change).
|
Thank you.
…On Tue, 10 Apr 2018 6:08 pm Drew Noakes ***@***.***> wrote:
***@***.**** approved this pull request.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4807 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEMyXlOaYVEVDLu_Yhmibao4G8YudZdRks5tnGh6gaJpZM4TLkHz>
.
|
What did I do to test the code and ensure quality:
Has been tested on (remove any that don't apply):