PikeNote / taskbar-groups-pike-beta

Lightweight application that lets users create and pin groups to the Windows taskbar och desktop
MIT License
126 stars 9 forks source link

Added Icon size and separation setting & elevated max shortcuts per group #35

Closed BanCrash closed 1 year ago

BanCrash commented 1 year ago

First example: imagen imagen

Second: imagen imagen

BanCrash commented 1 year ago

Hi @PikeNote , any thoughts about this?

PikeNote commented 1 year ago

@BanCrash Sorry about not being able to get back sooner! I think this is a great change for customizability purposes. My major concern with the pull is Hi-DPI support. What the code replaces is xDPI which helps adjust to Hi-DPI resolutions by default. I think the eDPI calculations should be default and let the user override that.

However, currently I need to re-examine the whole Hi-DPI fix, as the application right now does not work at higher scales or rather the UI breaks down rather badly.

I think I'll merge this in sometime down the line after I figure that out. The other concern which I will address sometime after this is pulled in is group editor clutter. There are currently a lot of options stacked on top of each other right now which isn't ideal. I may place some items side by side or have some kind of tab menu? Not sure where I would go but that is for the future.

BanCrash commented 1 year ago

@BanCrash Sorry about not being able to get back sooner! I think this is a great change for customizability purposes. My major concern with the pull is Hi-DPI support. What the code replaces is xDPI which helps adjust to Hi-DPI resolutions by default. I think the eDPI calculations should be default and let the user override that.

However, currently I need to re-examine the whole Hi-DPI fix, as the application right now does not work at higher scales or rather the UI breaks down rather badly.

I think I'll merge this in sometime down the line after I figure that out. The other concern which I will address sometime after this is pulled in is group editor clutter. There are currently a lot of options stacked on top of each other right now which isn't ideal. I may place some items side by side or have some kind of tab menu? Not sure where I would go but that is for the future.

Hi again, no worries, I asked just in case you didn't saw this PR.

About your comment, when I created the original commit there wasn't no DPI fix, and when making this PR I just checked that still compiled and can be able to merge but didn't realize that were changes on the original code. I'm surprised that this is still able to merging without conflicts even after those changes.

Anyway, I have to test it yet, but I think I could fix it to reaccept the DPI value, with something like this:

this.Height = (int)(loadedCat.IconSize + loadedCat.Separation * 2 * xDpi);

Like I've said I should test it before saying it's feasible but I think there shouldn't be any issues. And if that's the case we could have this PR without losing the DPI fix.

PikeNote commented 1 year ago

@BanCrash I just wrapped up with the last of the UI changes that I wanted to do to fix some Hi-DPI related issues. If you could, can you pull the changes to latest and resolve conflicts/ensure all controls are working as intended? Seems like there shouldn't be too much, and I'll review and pull in the changes.

Regarding the HiDPI fix, I think one other concern that I had just realized is that the icon scale would not change if the user changed scale/resolution after group creation. You could technically have it so that if the value was never changed, allow the application to calculate icon size at run time as the default behavior now. The only issue would be communicating that visually via the UI.

BanCrash commented 1 year ago

@BanCrash I just wrapped up with the last of the UI changes that I wanted to do to fix some Hi-DPI related issues. If you could, can you pull the changes to latest and resolve conflicts/ensure all controls are working as intended? Seems like there shouldn't be too much, and I'll review and pull in the changes.

Regarding the HiDPI fix, I think one other concern that I had just realized is that the icon scale would not change if the user changed scale/resolution after group creation. You could technically have it so that if the value was never changed, allow the application to calculate icon size at run time as the default behavior now. The only issue would be communicating that visually via the UI.

I will try to resolve the conflicts this weekend, I don't have much free time currently but I'll try.

I will check also if I can do something about the dpi thing.