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

Allow selecting custom icons for user defined scripts #11709

Merged
merged 8 commits into from
May 1, 2024

Conversation

mdonatas
Copy link
Contributor

@mdonatas mdonatas commented Apr 26, 2024

Proposed changes

  • In Scripts in addition to Icon property, add IconPathName in the property grid
    • First, this will allow to pick any icon the user wishes for
    • Second, this will allow picking high quality icons (built in ones are blurry)
  • Change icon related property handling such that their values are reflected in the scripts list immediately on-value-change
  • Fix DPI issues with script icons (see "before 200% scaling" screenshot)
  • Default position of PropertyGrid is ridonculous. StackOverflow listed the only option of using reflection to read into internal control and adjusting its splitter position.. not pretty but no downsides even if underling API changes and it looks much better.

Screenshots

Before

100% scaling
image

200% scaling
image

After

100% scaling
image

200% scaling
image

Test methodology

  • Manually by adding empty paths, incorrect paths, setting both Icon and IconPathName, etc.

Test environment(s)

  • Windows 11 200% scaling

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).


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

@mdonatas
Copy link
Contributor Author

It's hard to overstate how MUCH better script icons look in the toolbar with 4K screen with this change. This also made me realize that all other icons are of the CRT-monitor-smeared-with-greasy-fingers quality level 😅 but I don't know of an easy way to upgrade them.

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 nits, have not run

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.

have not run

@mdonatas
Copy link
Contributor Author

have not run

Thank you for the effort and the back-and-forth with me 🙏

@mdonatas
Copy link
Contributor Author

mdonatas commented Apr 27, 2024

Photo of the screen. Look at the bottom row and how blurry the icons are. Once I saw the custom icons selected for the scripts almost all other GE icons became perceptibly blurry once there was something to contrast them against.
They were added in the 6th commit 21b6b6a of this repo by @spdr870. Henk, do you remember the source of these icons? Maybe by sheer luck it would still be up and MAYBE have a higher quality icons too.

EDIT: Never mind, scratch that, all info was in that same mentioned commit, the website that had these icons is no longer working and I've only managed to find 16x16 icons https://www.flickr.com/photos/ddt3065snky/ These are "Xiao Icon" once hosted at ineversay.com

image

@mdonatas mdonatas requested a review from RussKie April 29, 2024 16:55
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

Few nits and naming feedback. Other than that - 👍

mdonatas and others added 2 commits May 1, 2024 18:17
Co-authored-by: Igor Velikorossov <RussKie@users.noreply.github.com>
@mstv mstv merged commit 9fc23fe into gitextensions:master May 1, 2024
4 checks passed
mstv pushed a commit to mstv/gitextensions that referenced this pull request May 5, 2024
* Allow selecting custom icons for user defined scripts
* Adjust PropertyGrid "properties" column width

Refs: gitextensions#11709
@RussKie RussKie added this to the vNext milestone May 21, 2024
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.

3 participants