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.

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

#9013 Use Close in Clean working directory #9027

Merged
merged 1 commit into from Apr 14, 2021
Merged

#9013 Use Close in Clean working directory #9027

merged 1 commit into from Apr 14, 2021

Conversation

bansan85
Copy link
Contributor

Fixes #9013

Proposed changes

  • Replace Cancel by Close to avoid confusion.

Screenshots

Before

image

After

image

Test methodology

Not needed

Test environment(s)

  • Git Extensions 33.33.33
  • Build dd34ed0 (Dirty)
  • Git 2.30.0.windows.1
  • Microsoft Windows NT 10.0.19042.0
  • .NET Framework 4.8.4341.0
  • DPI 96dpi (no scaling)

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

@ghost ghost assigned bansan85 Mar 21, 2021
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.

Looks good, just there are unneeded translation items...

@@ -3050,6 +3046,10 @@ revision:</source>
<source>Cleanup</source>
<target />
</trans-unit>
<trans-unit id="buttonClose.Text">
Copy link
Member

Choose a reason for hiding this comment

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

There is some way to avoid that this text (as well as the other using the new translated text), I do not recall how...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe remove the xx.Text="..." in FormCleanupRepository.Designer.cs and move it in the constructor of the form?

Copy link
Member

Choose a reason for hiding this comment

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

Should be OK, I believe there is some other trick too.
(This could maybe be fixed in a follow up, there are likely other similar strings.)

Copy link
Member

@pmiossec pmiossec Mar 21, 2021

Choose a reason for hiding this comment

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

Not in front of my computer, so you have to have a look at another form but to prevent the string to be translated, you should rename the button with the prefix looking link __no_translate

Copy link
Member

Choose a reason for hiding this comment

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

The prefix is _NO_TRANSLATE_.

@vbjay
Copy link
Contributor

vbjay commented Mar 21, 2021 via email

@bansan85
Copy link
Contributor Author

Thanks, I changed names.

@RussKie
Copy link
Member

RussKie commented Mar 21, 2021 via email

@bansan85
Copy link
Contributor Author

@RussKie Are you sure ? According to #9013 (comment) the question looks not answered. Would you like I remove the button only in "Clean working directory" ? Or also in "Reset changes" and "Recover lost objects" ?

@RussKie
Copy link
Member

RussKie commented Mar 21, 2021 via email

@gerhardol
Copy link
Member

For a dialog like this I believe that there should be a close button, nicr for the user than using the upper right close.

@RussKie
Copy link
Member

RussKie commented Mar 24, 2021

Other dialogs don't have "Close" button, and I can't see why this dialog needs it either.
"Cancel" button could be useful only if the clean operation could take significant time and can be cancelled. Even in this situation "Cancel" button must either be hidden or disabled until the operation is in flight.

@RussKie RussKie merged commit 187395a into gitextensions:master Apr 14, 2021
@ghost ghost added this to the 3.6 milestone Apr 14, 2021
@RussKie
Copy link
Member

RussKie commented Apr 14, 2021

Thank you

@bansan85
Copy link
Contributor Author

Thanks, I didn't know what to do with this PR since some people wanted to remove this button and some other don't wanted.

@RussKie
Copy link
Member

RussKie commented Apr 14, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove "Close"/"Cancel" button in Form that just close the windows
6 participants