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

Added toolStripFileExplorer to complete the feature-request #4018

Merged
merged 3 commits into from
Sep 23, 2017
Merged

Added toolStripFileExplorer to complete the feature-request #4018

merged 3 commits into from
Sep 23, 2017

Conversation

joshia89
Copy link

Fixes #3585 .

Changes proposed in this pull request:

  • Add a "File Explorer" shortcut in the toolbar (next to "Git bash")

Screenshots before and after (if PR changes UI):

image

How did I test this code:

  • Make sure the button appears correctly
  • If it is a valid working directory, the file explorer is enabled and behaves as expected
  • If it is not a valid working directory, the file explorer button is disabled

@@ -1902,6 +1903,18 @@ private void ToolStripButtonPushClick(object sender, EventArgs e)
PushToolStripMenuItemClick(sender, e);
}

private void ToolStripFileExplorerClick(object sender, EventArgs e)
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 already a handler doing just this - FileExplorerToolStripMenuItemClick
please bind the event to the existing handler

this.toolStripFileExplorer.ImageTransparentColor = System.Drawing.Color.Gray;
this.toolStripFileExplorer.Name = "toolStripFileExplorer";
this.toolStripFileExplorer.Size = new System.Drawing.Size(23, 22);
this.toolStripFileExplorer.ToolTipText = "File Explorer";
Copy link
Member

Choose a reason for hiding this comment

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

this text needs to be translated

the easiest way is probably do the following:

        private FormBrowse()
        {
            InitializeComponent();
            Translate();

            toolStripFileExplorer.ToolTipText = fileExplorerToolStripMenuItem.Text;
        }

@jbialobr what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

ToolTipTexts assigned this way are translated automagically. #3745

Copy link
Member

Choose a reason for hiding this comment

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

For the automagic to work, someone will still need to provide translations, won't they?
fileExplorerToolStripMenuItem performs exactly the same function and has the required text, which is already translated. Hence my suggestion to copy and assign the existing translation to the new element.

...Or does the automagic work somehow differently?

Copy link
Member

Choose a reason for hiding this comment

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

For the automagic to work, someone will still need to provide translations, won't they?

Yes, that is true. But I would not mix Text and ToolTipText. It so happened they share the same text, but it should be handled on the translation side to prompt the same translation for the same texts when a translator is translating.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @RussKie and @jbialobr. RussKie, do you want me to make any changes here?

P.S the other requested change has been committed

@RussKie
Copy link
Member

RussKie commented Sep 23, 2017 via email

@jbialobr jbialobr merged commit e09b1ac into gitextensions:master Sep 23, 2017
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.

None yet

3 participants