Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dialogs layout alignment #8292

Merged
merged 2 commits into from Jul 12, 2020
Merged

Dialogs layout alignment #8292

merged 2 commits into from Jul 12, 2020

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Jul 5, 2020 •
edited

Relates to #6183

Proposed changes

  • Add a new base dialog control GitExtensionsDialogthat features two main sections - MainPanel for the main content, and ControlPanel for all buttons. All form should ideally inherit from this dialog to provide a consistent look and feel.
  • Update FormInit layout
  • Update FormClone layout
  • Update FormStatus layout
  • Update FormProgress layout
  • Update FormRemoteProgress layout

⚠️ This work concentrates only on the placement of buttons, the original layout of dialogs is preserved (for the most part), and changes to the actual layout of each individual form is outside the scope of this work.

Screenshots

Before

  • FormInit
    image

  • FormClone
    image

  • FormClone
    image

After

  • FormInit
    image
    image

  • FormClone
    image
    image

  • FormClone
    ge-status-dark
    ge-status-light

Test environment(s)

  • Microsoft Windows NT 10.0.18363.0
  • .NET Framework 4.8.4180.0
  • DPI 144dpi (150% scaling)

✒️ I contribute this code under The Developer Certificate of Origin.

@ghost ghost assigned RussKie Jul 5, 2020
gerhardol
gerhardol reviewed Jul 5, 2020
View reviewed changes
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Briefly looked at it, no comments
Is the layout with PuTTY considered?

@RussKie
Copy link
Member Author

RussKie commented Jul 5, 2020 •
edited by NikolayXHD

@NikolayXHD I've compared colours in Windows TaskDialog implementation, and it uses slightly different colours to what defined by the .NET runtime. Specifically the shades of the bottom panel:
(left a TaskDialog, right .NET Fx)

The native control uses #f0f0f0, and .NET Fx ControlLight is #e3e3e3. I'd like to define a custom colour via theme, but I'm a little stumped on how to best achieve it. Do I make it a part of AppColor set?

I am not sure I understand your goal.

My understanding is that you want to make the dialogs modified in this PR match the colors of Windows TaskDialog, and Windows TaskDialog bottom panel is #f0f0f0

If that's correct, then our dialogs should use for bottom panel either one of

  • SystemColors.Control
  • SystemColors.Menu
  • SystemColors.MenuBar

These are all equal #f0f0f0 in system-defined light theme:

lookup f0f0f0 in https://github.com/gitextensions/gitextensions/blob/master/GitUI/Themes/win10default.css

I would pick SystemColors.Control for this.

Sorry @RussKie I messed up, edited your comment instead of replying :(

@RussKie
Copy link
Member Author

RussKie commented Jul 5, 2020

Is the layout with PuTTY considered?

Yep
image

mstv
mstv reviewed Jul 5, 2020
View reviewed changes
Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit regarding readability

GitUI/CommandsDialogs/FormInit.cs Outdated Show resolved Hide resolved
@gitextensions gitextensions deleted a comment from codecov bot Jul 5, 2020
@gitextensions gitextensions deleted a comment from codecov bot Jul 5, 2020
@RussKie RussKie mentioned this pull request Jul 8, 2020
Merged
@codecov
Copy link

codecov bot commented Jul 10, 2020 •
edited

Codecov Report

Merging #8292 into master will increase coverage by 0.02%.
The diff coverage is 87.09%.

@@            Coverage Diff             @@
##           master    #8292      +/-   ##
==========================================
+ Coverage   52.65%   52.67%   +0.02%     
==========================================
  Files         863      864       +1     
  Lines       62491    62509      +18     
  Branches    11196    11196              
==========================================
+ Hits        32902    32924      +22     
+ Misses      26966    26964       -2     
+ Partials     2623     2621       -2
Flag Coverage Δ
#production 40.67% <87.09%> (+0.02%) ⬆️
#tests 94.46% <ø> (+0.02%) ⬆️

@RussKie
Copy link
Member Author

RussKie commented Jul 11, 2020

FormStatus change:

ge-status-dark
ge-status-light

@mstv
Copy link
Member

mstv commented Jul 11, 2020

Rebase, please, in order to resolve conflicts and for easier review.

@RussKie
Copy link
Member Author

RussKie commented Jul 11, 2020

I'd like to have this merged asapp, I'm cleaning the FormStatus API (it is mind boggling spaghetti), and it depends on this.

mstv
mstv approved these changes Jul 11, 2020
View reviewed changes
Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, have not run it.

GitUI/HelperDialogs/FormProcess.cs Outdated Show resolved Hide resolved
GitUI/HelperDialogs/FormStatus.cs Outdated Show resolved Hide resolved
GitUI/HelperDialogs/FormStatus.cs Outdated Show resolved Hide resolved
GitUI/UserControls/RevisionGrid/RevisionGridControl.cs Outdated Show resolved Hide resolved
@RussKie
Dialogs layout alignment - FormInit and FormClone
ed70cd8 
* Add a new base dialog control `GitExtensionsDialog`that features two main
sections - `MainPanel` for the main content, and `ControlPanel` for all buttons.
All form should ideally inherit from this dialog to provide a consistent look
and feel.
* Update `FormInit` layout
* Update `FormClone` layout

Relates to gitextensions#6183
@RussKie
Dialogs layout alignment - FormStatus and descendants
12c2310 
* Update `FormStatus` layout
* Update `FormProgress` layout
* Update `FormRemoteProgress` layout

Relates to gitextensions#6183
@RussKie RussKie merged commit 316f8d4 into gitextensions:master Jul 12, 2020
@ghost ghost added this to the 4.0 milestone Jul 12, 2020
@RussKie RussKie deleted the fix_6183_design branch July 12, 2020 12:04
@mstv mstv mentioned this pull request Jul 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gerhardol gerhardol gerhardol left review comments

@mstv mstv mstv approved these changes

Assignees

@RussKie RussKie

Labels
area: user experience
Projects
None yet
Milestone
3.5
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@RussKie @mstv @gerhardol