search

aRTy42 / POE-ItemInfo

Item Info Script for Path of Exile
167 stars 225 forks source link

SettingsUI Controls overlapping #52

Closed 4GForce closed 7 years ago

4GForce commented 7 years ago

Mostly in the tooltip section, i'm working on it.

4GForce commented 7 years ago

From what I've read this gui can be used within tabs, this would require testing if you can point me to that scenario. ( I use gui margin positionning in some cases )

Fixed

4GForce commented 7 years ago

Well, I found out that the gui is used in a tab within TradeMacro and margin positionning breaks it. I'll work on fixing this.

Eruyome commented 7 years ago

Yeah, the positioning is a bit complicated, I wasn't aware of any problems though the last time I worked on it to make it tab-compatible for TradeMacro.

4GForce commented 7 years ago

-1 to ahk for form building The problem might not appear to everyone due to screen res, windows theme, etc But I clearly had overlapping controls in the tooltip section and I'll do my best to fix it but with stuff like groupboxes not even acting as container and more, I can't guarantee 100% quality on every systems.

Eruyome commented 7 years ago

Could you show me a screenshot of this mentioned overlapping?

aRTy42 commented 7 years ago

I've reverted the merge. Could you two check if the current state of the repo is working for you regarding the encoding issues, please.

Eruyome commented 7 years ago

It seems to work. Both ItemInfo and TradeMacro.

Btw, I'll say it again, when merging PRs you should use Squash and merge instead of merge. Causes less clutter in the commit history ;-) Makes sense for normal commits too to squash a few together.

aRTy42 commented 7 years ago

I misunderstood your squashing comment from last time, I thought that was only an option when using the desktop version or command line and I usually use the web version. I looked it up again and changed the setting for the repo to only allow squash-merge now, so I will remember next time.

Eruyome commented 7 years ago

Yeah, could have been a bit more clear with my explanation. I don't always use squash but at least when I have a few commits lined up from my dev branch and merge it to my master branch. Have only used it via the web ui myself.

I also try to squash only commits that are somewhat related or just minor things. Seperating bigger changes is probably a good idea.

4GForce commented 7 years ago

New TradeMacro 1.7.2 release seems to be working fine. And here are the screenshots of the UI, its minor but I felt like fixing it.

In the tooltip section

iteminfosettingsui

In the hotkey section

trademacrosettingsui

4GForce commented 7 years ago

Quick fix, nothing fancy this time and I made sure it was working with TradeMacro tab control.

iteminfosettingsui_fixed

Eruyome commented 7 years ago

You can't really fix that overlapping in the TradeMacro Hotkey section, those checkboxes expect some text after the box and it doesn't seem like you can reduce the elements width to only have the size of the box. Therefore this invisible/non-existing text overlaps the border. I decided to just keep it that way.

aRTy42 commented 7 years ago

You could move the text fields and checkboxes a little to the left. With GuiAddEdit x+5 and GuiAddCheckbox x+3 it works.

You could also make the text fields a bit smaller.

4GForce commented 7 years ago

Well I still haven't figured out how to properly commit changes to TradeMacro branch but changing those editboxes from 'x+10' to 'x+1' sure solves the problem, from my perspective at least.

Eruyome commented 7 years ago

Is it so hard to properly commit to TradeMacro?^^

Fork the latest version, make some changes, push them cross-fork (pull request) while selecting the TradeMacro master branch as HEAD.

Can't be any different then making changes and PRs to ItemInfo, or am I wrong?

4GForce commented 7 years ago

Well, when I click fork from TradeMacro repo it brings me to my own ItemInfo repo fork https://github.com/4GForce/POE-ItemInfo ?!

I don't want to delete my ItemInfo fork because I have a pending pull request and from there I don't even see the TradeMacro files ... maybe I'm doing something wrong ?!?

Eruyome commented 7 years ago

Well, maybe its a bit more complicated since TradeMacro itself is a forked repo. Maybe I'll make a second github account and test it a bit.

4GForce commented 7 years ago

I'll try deleting my ItemInfo fork once the pull request has passed and then fork directly from TradeMacro.

Eruyome commented 7 years ago

It seems the ItemInfo fork is a problem, worked with a new github account without any forks. There's probably a way to have both repos forked though.

aRTy42 commented 7 years ago

merged your pull request, 4G