[Qt] fix icon sizes and column width #5626

jonasschnelli commented at 10:56 AM on January 9, 2015: contributor

No description provided.

laanwj added the label GUI on Jan 9, 2015

jonasschnelli commented at 10:59 AM on January 9, 2015: contributor

Will look like:

win: bildschirmfoto 2015-01-09 um 11 55 27

osx: bildschirmfoto 2015-01-09 um 11 58 56

paveljanik commented at 11:28 AM on January 9, 2015: contributor

On OS X 10.10, Retina: STATUS_COLUMN_WIDTH = 30: status_column_width30 STATUS_COLUMN_WIDTH = 40: status_column_width40

jonasschnelli commented at 12:16 PM on January 9, 2015: contributor

@paveljanik as mentioned on IRC: please don't use Qt4 anymore for OSX builds. It doesn't support retina.

We even should drop Qt4 compatibility for OS X because it will end up with large icons as shown above.

luke-jr commented at 12:17 PM on January 9, 2015: member

A warning at the end of configure would be appropriate, but breaking it for no reason would just be annoying IMO. Some users probably don't have Retina, and wouldn't benefit from updating Qt.

jonasschnelli commented at 12:19 PM on January 9, 2015: contributor

@luke-jr even non retina mac and older mac could run Qt5 binaries. With Qt5 the oversized icon would also render nice on non-retina. Deprecated-unsupported warning is fine for me.

jonasschnelli commented at 10:08 AM on January 12, 2015: contributor

@paveljanik could you retest this with Qt5 already?

jonasschnelli commented at 5:55 AM on January 13, 2015: contributor

Regarding sizes: please see #5489

jonasschnelli commented at 8:07 AM on January 13, 2015: contributor

@zander Transparent space in icons make icons look more uniform when displayed together without clustering the code with some magic padding numbers. And potential replacement would also be more complicated with static padding in the code.

zander commented at 8:59 AM on January 13, 2015: none

Transparent space in icons make icons look more uniform when displayed together

Definitely!

without clustering the code with some magic padding numbers. And potential replacement would also be more complicated with static padding in the code.

Don't use static padding, just Qt::AlignmentCenter

Regarding sizing; you should really run optimize on the images before you merge, so you don't end up changing it twice. Git will keep all versions on everyones harddrive forever. Best to avoid unneeded changes. 15Kb vs 700 bytes is two orders of magnitude...

jonasschnelli commented at 9:13 AM on January 13, 2015: contributor

Don't use static padding, just Qt::AlignmentCenter

But this would not solve uniforming sized between icons.

Regarding sizing; you should really run optimize on the images before you merge, so you don't end up changing it twice. Git will keep all versions on everyones harddrive forever. Best to avoid unneeded changes. 15Kb vs 700 bytes is two orders of magnitude...

Yes. But this time we encountered the size issue after the merging of some new icons. That's why we have now #5489 with a included script so we might run it next time before creating a PR or merge.

zander commented at 10:05 AM on January 13, 2015: none

Yes. But this time we encountered the size issue after the merging of some new icons. That's why we have now #5489 with a included script so we might run it next time before creating a PR or merge.

You say "yes", but I don't see a new push, so you really meant "no", didn't you?

Why don't you update this merge with the fixed icons?

jonasschnelli commented at 10:15 AM on January 13, 2015: contributor

@zander Because #5489 is already open and has discussion and is not finished yet.

jonasschnelli force-pushed on Jan 14, 2015

jonasschnelli commented at 4:10 PM on January 14, 2015: contributor

optimized the two pngs in this pull according to #5489.

fanquake commented at 10:35 AM on January 16, 2015: member

Needs rebase

[Qt] resize oversized icons af95b17d0d

[Qt] change transaction table column width

make enought space for the new icons

652eb904df

jonasschnelli force-pushed on Jan 16, 2015

jonasschnelli commented at 10:48 AM on January 16, 2015: contributor

Rebased.

fanquake commented at 11:03 AM on January 16, 2015: member

Tested ACK OSX 10.9 with Qt5

laanwj commented at 11:30 AM on January 16, 2015: member

This makes the status column too wide here (Ubuntu Qt5). This is not really a problem, but looks asymmetric, because the column is not centered.

untitled

jonasschnelli commented at 1:02 PM on January 16, 2015: contributor

@laanwj My ubuntu 14.04 gives out a more or less centered column. bildschirmfoto 2015-01-16 um 14 00 51

will try now to center the content in the column.

jonasschnelli commented at 1:17 PM on January 16, 2015: contributor

Centering the QIcon within the QTableWidgetCell would need a custom class with a QPainter (see http://qt-project.org/forums/viewthread/31839). Just setting the alignment did not work.

I don't think it's worth doing this.

laanwj commented at 1:53 PM on January 16, 2015: member

Blegh. Can we make the width adaptive somehow? I don't understand why it shows ~~narrower~~ wider here.

laanwj referenced this in commit 29948be957 on Jan 18, 2015

jonasschnelli commented at 1:44 PM on January 20, 2015: contributor

@laanwj: Currently i'm not investigating the width issue. This pull make sense because the current master looks like this on windows: bildschirmfoto 2015-01-20 um 14 37 11

laanwj commented at 8:54 AM on January 21, 2015: member

But it's weird isn't it? The image has a fixed width in pixels. We set a fixed width in pixels for the column. How can this differ so much per OS or seemingly even per theme :/

laanwj merged this on Jan 26, 2015

laanwj closed this on Jan 26, 2015

laanwj referenced this in commit ab0d7987c0 on Jan 26, 2015

MarcoFalke locked this on Sep 8, 2021