#12978 closed bug (fixed)
Tracker Get Info window layout problem
Reported by: | jackburton | Owned by: | axeld |
---|---|---|---|
Priority: | normal | Milestone: | Unscheduled |
Component: | Applications/Tracker | Version: | R1/Development |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Platform: | All |
Description (last modified by )
Attachments (3)
Change History (15)
by , 8 years ago
Attachment: | Tracker Get Info.png added |
---|
comment:1 by , 8 years ago
Description: | modified (diff) |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 8 years ago
Replying to diver:
what font changes?
I've only been paying a little attention to the mailing list and I noticed there's been a great deal of font change discussions, and I was assuming that had been the cause when I couldn't replicate it with a few quick font size tests (18 & 16).
The rendering of the font in the screenshot reinforced that, as it's not very sharp.
The issue does seem to be a matter of calculating the spacing and positioning due to font size changes, but not anything new.
by , 8 years ago
Attachment: | 0001-Fix-for-ticket-12978-Tracker-GetInfo-window-layout-p.patch added |
---|
comment:5 by , 8 years ago
patch: | 0 → 1 |
---|
comment:6 by , 8 years ago
I just submitted my first patch. I tried to follow the directions. I hope it is useful.
follow-up: 8 comment:7 by , 8 years ago
I haven't tested if the patch actually fixes the issue, but it looks good, although I would use "++" instead of "+= 1".
by , 8 years ago
Attachment: | 0002-Fix-for-ticket-12978.patch added |
---|
comment:8 by , 8 years ago
Replying to axeld:
I haven't tested if the patch actually fixes the issue, but it looks good, although I would use "++" instead of "+= 1".
Thank you for the quick feedback. I have attached a 2nd patch for that recommendation.
comment:9 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | assigned → in-progress |
follow-up: 11 comment:10 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | in-progress → closed |
Applied in hrev50847, thanks for the patch!
I've merged the second patch into the first one -- as long as it hasn't been committed, it's preferred to send in a new patch instead. I've one question, though: why lines * 2 + 1? That sounds a bit magical to me.
comment:11 by , 8 years ago
Replying to axeld:
Applied in hrev50847, thanks for the patch!
I've merged the second patch into the first one -- as long as it hasn't been committed, it's preferred to send in a new patch instead. I've one question, though: why lines * 2 + 1? That sounds a bit magical to me.
I tried to combine the two patches, but got frustrated. I am a newbie with git.
The original code was:
float height = font->Size() * 15;
I reversed engineered that into 7 double spaced lines plus one extra to make the "Permissions" fit properly in the window.
I have reviewed the coding guidelines again and realize that I should have used an "if ... else if ..." in order to match the code that draws the lines. I had it that way originally and changed it because I misread the guidelines for "if/else if" with returns.
comment:12 by , 8 years ago
Indeed, I missed the missing else
-- it's not time critical code, though, so it's not that much of an issue.
There are several ways to combine patches with git, the easiest one would be if your last patch is still "pending": then you can just commit the changes with git commit --amend
, and your pending patch is merged with the new changes.
Thanks for the explanation!
I believe this may be a bug related to recent font changes.As of HREV 50391, this bug does not exist:EDIT:
Disregard, certain font size selections can cause it. I though I matched the size in the screenshot, but I clearly did not.
Will dig deeper next week.