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

DolphinQt: Rewrite 'About' dialog to match the new WX one. #2833

Merged
merged 1 commit into from Sep 12, 2015

Conversation

waddlesplash
Copy link
Contributor

Current WX one:

oldwx

New Qt one:
newqt

@waddlesplash
Copy link
Contributor Author

Review? @lioncash a/o @endrift

@lioncash
Copy link
Member

looks fine

With the Qt one, I think the right side of the dolphin should have a slight bit more padding on it.

@waddlesplash
Copy link
Contributor Author

oh, also cc @MaJoRoesch

Thanks to hcs/destop for their GC ADPCM decoder.<br>
<br>
We are not affiliated with Nintendo in any way. GameCube and Wii are&lt;br&gt; trademarks of Nintendo. This emulator should not be used to play games&lt;br&gt;you do not legally own.</string>
<string>&lt;a href=&quot;https://github.com/dolphin-emu/dolphin/blob/master/license.txt&quot;&gt;License&lt;/a&gt; | &lt;a href=&quot;https://github.com/dolphin-emu/dolphin/graphs/contributors&quot;&gt;Authors&lt;/a&gt; | &lt;a href=&quot;https://forums.dolphin-emu.org/&quot;&gt;Support&lt;/a&gt;</string>

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@tony971
Copy link

tony971 commented Aug 12, 2015

I know that the reason "Dolphin" is slightly offset right in the WX version is because they couldn't get it to line up with the coding. Is this also the case with Qt or did you just want it to match WX exactly?

@endrift
Copy link
Contributor

endrift commented Aug 12, 2015

I haven't taken a close look at this yet. Does it properly handle @2x on the logo?

@endrift
Copy link
Contributor

endrift commented Aug 12, 2015

Also, the logo needs better padding around it. The text is way too tight next to it.

@MayImilae
Copy link
Contributor

DEFINITELY increase the padding around the logo, and increase the spacing between the lines. And you may want to reduce the size of the text that runs along the bottom, it's kind of using the enter horizontal width, which looks weird.

@waddlesplash
Copy link
Contributor Author

Is this also the case with Qt or did you just want it to match WX exactly?

No, I wanted it to look as good as possible, not just "match WX exactly".

I haven't taken a close look at this yet. Does it properly handle @2x on the logo?

Yes, the "Resources" class already handles @2x image loading. (try it on HiDPI and see for yourself).

DEFINITELY increase the padding around the logo, and increase the spacing between the lines. And you may want to reduce the size of the text that runs along the bottom, it's kind of using the enter horizontal width, which looks weird.

OK, will do.

@ghost
Copy link

ghost commented Aug 12, 2015

I think the copyright line in the WX version still looks a bit better as far as spacing and font size goes. Could you try to match that?

@ghost
Copy link

ghost commented Aug 12, 2015

Also in my original mockup I was looking to have it so the links were only underlined on mouseover but I think it was determined that wasn't possible with WX. Can that be done with Qt and if so would you considering adding it?

For reference this was the look I was going for when this was first done:

68747470733a2f2f692e696d6775722e636f6d2f564a493462725a2e706e67

@waddlesplash
Copy link
Contributor Author

Better?

@waddlesplash
Copy link
Contributor Author

Also in my original mockup I was looking to have it so the links were only underlined on mouseover but I think it was determined that wasn't possible with WX. Can that be done with Qt and if so would you considering adding it?

Qt's CSS styling doesn't support "text-decoration", unfortunately. So, not easily.

@MayImilae
Copy link
Contributor

It could still use some more padding between the logo and the text... And the spacing between the rows of text is really really tight. Aaaand the license | support | authors looks better in WX, thanks to some extra spacing.

The copyright text is good though! :P

I guess the best advice I can give is to point to the WX version and say "do that"! It's one of the few pieces of the WX GUI that's actually been thoroughly reviewed and tweaked recently, so it's actually pretty good.

EDIT: Btw, I don't know why, but your WX current image is kind of weird... the WX about window looks better on when I try it on Windows. Perhaps it's your distro? shrug

aboutwx

@waddlesplash
Copy link
Contributor Author

Aside from all the insane misalignment in KDE, you mean. 😛

@MayImilae
Copy link
Contributor

I guess someone needs to upload a screenshot of this PR from windows (or a distro without alignment issues) then. Please don't make the dialup girl do it. :'(

@NewLunarFire
Copy link

From a pure outsider POV, I have to disagree with you MaJoRoesch. I think the hierachy and spacing of the blocks look better on waddlesplash example. The build information is clearer when made a little bigger and it properly separates Build Info, Check for Updates, Disclaimers and links blocks.

I don't know if my intervention has it's place here or if it should be discussed elsewhere thought, but I think if we want to rewrite this dialog we might as well make sure it looks right.

@JosJuice
Copy link
Member

Here's the current version of this PR in Windows 7.

  • The question mark button doesn't do anything useful.
  • Why does the link have a special background color?
  • Fields like the revision can't be selected and copied. We agreed that it would be nice to have when the wx About dialog was being made, but maybe it's hard to implement or gives a bad result.

@RisingFog
Copy link
Member

@JosJuice we eventually fully reverted from that as the implementation was simply not possible to work on every OS combination.

@BhaaLseN
Copy link
Member

...which is why I didn't ask for it (yet). Was going to poke waddle on IRC before putting it here, but kinda forgot about that :S

@endrift
Copy link
Contributor

endrift commented Aug 13, 2015

I know it's possible to make text in a dialog selectable because I did it in mGBA's about dialog.

@ghost
Copy link

ghost commented Aug 13, 2015

I think the window could use less space beneath the copyright line. Also the spacing doesn't look properly centred in @JosJuice's image. Is the copyright symbol being ignored for formatting in Windows for some reason?

@waddlesplash
Copy link
Contributor Author

The question mark button doesn't do anything useful.

Offline WIP on my end removes it, yeah.

Why does the link have a special background color?

I was copying WX. Looks like it was just a focus ring. Removing.

Fields like the revision can't be selected and copied. We agreed that it would be nice to have when the wx About dialog was being made, but maybe it's hard to implement or gives a bad result.

It's doable. Will do.

@waddlesplash
Copy link
Contributor Author

Also the spacing doesn't look properly centred in @JosJuice's image. Is the copyright symbol being ignored for formatting in Windows for some reason?

No, looks like I messed up the spacing hints on the last round of changes.

@@ -1,4 +1,4 @@
// Copyright 2014 Dolphin Emulator Project
// Copyright 2015 Dolphin Emulator Project

This comment was marked as off-topic.

@waddlesplash
Copy link
Contributor Author

@everyone: All comments addressed. Here's what it looks like now:

@waddlesplash waddlesplash force-pushed the dolphin-qt branch 2 times, most recently from add0493 to e6e7f77 Compare August 13, 2015 19:59
@waddlesplash
Copy link
Contributor Author

@JosJuice: fixed.

@endrift
Copy link
Contributor

endrift commented Aug 13, 2015

Logo might need a speck more padding, but otherwise LGTM.

@JosJuice
Copy link
Member

Looks nice here.

@RisingFog
Copy link
Member

LGTM, might revise the WX splash to put the bottom sentences together as it looks nicer.

@ghost
Copy link

ghost commented Aug 13, 2015

I'm not really a fan of how thick the font is for the title in Windows and I also think there could still be a bit more padding around the logo but otherwise LGTM.

@MayImilae
Copy link
Contributor

I agree with Pringo, some more padding around the logo would be nice. I like how the WX version is padded, and if you see about dialogs in firefox and VLC they are done in a similar style with a lot of padding around the logo. It gives the logo a great presence in the dialog and a very pleasing composition overall. Let me see if I can show you what I'm seeing...


aboutwx2

See how consistent the padding is here? Also the Dolphin isn't filling the entire box, and the distance from the logo's tip to the edge of the box, and the distance from the edge of the box to the text/edge of the frame, are all about the same. Composition is all about the arrangement and alignment of details like this within the frame! And how even and nice everything is in the WX version really helps the logo feel like it's cozy in that space.


aboutqt2

In the Qt version, the logo almost completely fills the box, which is unfortunate. And the padding around the box is very inconsistent, with the left having much less than the other sides. All combined, even though it is the box is a perfect square, it doesn't look like one, it looks taller than it is wide. It's just an illusion of course, but it results in a kind of claustrophobic feeling to the logo, like it's not big enough for the space it's in, like it was smushed into place. Also, the whole window is narrow on the right side as well, closing in very tightly on the "own." to the right. It just needs to be wider, imo. It's issues like this that I think are why it isn't as good compositionally.


And like Pringo, I don't like the bold style for the Dolphin text, at least how it appears on Windows. It looks fine in KDE of course, but you know, operating systems and fonts are weird!

But the spacing of the text itself overall however is much better!


Oh, I'm curious, why did you remove the spacing between "Dolphin is a free and open source GameCube and Wii emulator" and "This software should not be used to play games you do not legally own"? They were separated before because they are separate thoughts, but in comparison to the branch/revision/compiled section one could consider them as "grouped" in a similar way. I'm not super attached to it, but it kind of bothers me... With them so close is that it theoretically connects the emulator with piracy; the separation in the WX version kind of makes it clear that the emulator and "do not do piracy" are separate concepts.
In a very very minor note, I still prefer the slight additional horizontal spacing between License | Authors | Support that is in the WX version. That's an extreme nitpick though...
@NewLunarFire I don't really care about the text size. :P And my point had nothing to do with that! I'm more worried about the composition and grouping of the items, personally. The bigger text is fine with me, since the grouping of lines takes care of any comprehension issues.

@waddlesplash
Copy link
Contributor Author

@delroth Can't you somehow blacklist changes to DolphinQt/DolphinWX as not ever impacting graphical rendering? The bot is kinda annoying (and it's wasting time/money...)

@waddlesplash
Copy link
Contributor Author

@zopieux done.

@MaJoRoesch et al, here's what it looks like now (Qt on top, WX on bottom):
1

@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • chibi-robo-fastdepth on dx-win-nv: diff
  • chibi-robo-zfighting on dx-win-nv: diff
  • djfny-menu on dx-win-nv: diff
  • ea-vp6 on dx-win-nv: diff
  • ed-lighting on dx-win-nv: diff
  • fifa-street on dx-win-nv: diff
  • find-mii on dx-win-nv: diff
  • fog-adj on dx-win-nv: diff
  • fortune-street on dx-win-nv: diff
  • fortune-street-fog on dx-win-nv: diff
  • fortune-street-white-box on dx-win-nv: diff
  • inverted-depth-range on dx-win-nv: diff
  • kirby-shadows on dx-win-nv: diff
  • line-width-test on dx-win-nv: diff
  • luigi-shadows on dx-win-nv: diff
  • mario-sluggers-bar on dx-win-nv: diff
  • mario-tennis-menu on dx-win-nv: diff
  • megaman-heat on dx-win-nv: diff
  • melee-lighting on dx-win-nv: diff
  • mii-channel on dx-win-nv: diff
  • mini-ninjas on dx-win-nv: diff
  • mkdd-efb on dx-win-nv: diff
  • mkwii-bluebox on dx-win-nv: diff
  • monkeyball-fuse on dx-win-nv: diff
  • mp7-text on dx-win-nv: diff
  • mtennis-zfreeze on dx-win-nv: diff
  • my-word-coach on dx-win-nv: diff
  • nfsu-purplerect on dx-win-nv: diff
  • nfsu-reflections on dx-win-nv: diff
  • nsmbw-intro on dx-win-nv: diff
  • rs2-glass on dx-win-nv: diff
  • rs2-zfreeze on dx-win-nv: diff
  • sadx-ui on dx-win-nv: diff
  • sf-assault-flashing on dx-win-nv: diff
  • simpsons-tev on dx-win-nv: diff
  • smg2-fog on dx-win-nv: diff
  • smg-marioeyes on dx-win-nv: diff
  • sms-bubbles on dx-win-nv: diff
  • soa-black on dx-win-nv: diff
  • ssbm-pointsize on dx-win-nv: diff
  • ss-timestone on dx-win-nv: diff
  • sw3-dt on dx-win-nv: diff
  • thps4-shadow on dx-win-nv: diff
  • tos-invis-char on dx-win-nv: diff
  • tsp3-pinkgrass on dx-win-nv: diff
  • xenoblade-menu on dx-win-nv: diff
  • ztp-grass on dx-win-nv: diff
  • zww-armos on dx-win-nv: diff
  • zww-water on dx-win-nv: diff
  • zww-waves on dx-win-nv: diff

automated-fifoci-reporter

@MayImilae
Copy link
Contributor

Much better! I approve. Can we get a pic of this on windows?

Edit: Whoops! That was windows!

lioncash added a commit that referenced this pull request Sep 12, 2015
DolphinQt: Rewrite 'About' dialog to match the new WX one.
@lioncash lioncash merged commit 4de2bd3 into dolphin-emu:master Sep 12, 2015
@JosJuice JosJuice mentioned this pull request Feb 12, 2016
@leoetlino leoetlino modified the milestone: Qt Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet