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

a bunch of fixes and improvements, fix #39 #40 #42 #43

Open
wants to merge 16 commits into
base: master
from

Conversation

@illwieckz
Copy link
Member

commented Aug 17, 2019

medium thumbnail not always exist, the full one is expected to always exist, fix #39

we can display the blog post even if the thumbnail fails to load, fix #40

it's better to tell people what to do, fix #42

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2019

What I see is that the eight of the text zone is the eight of the image, which looks a bit inconsistent across news. If I'm right the medium thumbnail is cropped to a square (there is an option for that Wordpress side). But that's a minor issue that is far more acceptable than news that don't load at all.

updater

updater

updater

The other issue is of course that there is more downloaded data.

I really don't know how to do “try medium then full” in that code.

@illwieckz illwieckz force-pushed the fullthumbnail branch from a08e7f2 to cafc9c2 Aug 19, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

OK, I found the way to attempt to load the medium one then to fallback on full one.

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

I did a large rework of news stuff

  • display the text even if the thumbnail is still loading,
    makes sure the news is readable and link clickable even
    if thumbnail fails to load
  • use a fallback thumbnail if json does not provide it,
    use unvanquished icon
  • always load full thumbnails,
    it looks far better
  • use constant size for thumbnail and text zone
    it looks more consistent accross news
  • crop thumbnail within updater
    required since thumbnail is displayed with custom size
  • clicking on thumbnail also open news
    it's handy, and can be used as a fallback for link
  • never overlap page indicator on thumbnail and text
  • make page indicator visible over dark background and clickable,
    it works!
  • resize title and body news font
    it fits more the text zone
  • bold title font
  • make grey body news font to make it more readable (less contrasted)
  • add a subtle margin around text to make it more elegant
  • clip the text (do not print outside of text zone)

Thumbnail quality before:

unvanquished updater

Thumbnail quality now:

unvanquished updater

Loading thumbnail (it keeps this state if it fails to download, I tested wrong url which is taken as a wrong image format, and network disconnection):

unvanquished updater

Missing thumbnail (must not happen, but we are ready):

unvanquished updater

@illwieckz illwieckz force-pushed the fullthumbnail branch from c44e9da to 1e7fb76 Aug 19, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

I fount a way to also display fallback on thumbnail error

the rotating loading symbol is displayed until the fallback is loaded or fails to load, then the thumbnail is displayed or the fallback is printed instead

it is known to behave this way on wrong url, bad image format, and network outage

@illwieckz illwieckz force-pushed the fullthumbnail branch 3 times, most recently from f0bd948 to dc7f20e Aug 19, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

The updater also displays a fallback post if json fails to load.

I verified it works on disconnected network, unresolved domain name, or 404 error, there code also take care of bad json but that part does not work, probably due to a Qt bug (they even returns HTTP 200 code on 301 redirect… so I would not be surprised if there would be other bugs).

By the way the fallback post looks like this, I just reused the words I've recently written on home page:

unvanquished updater

The link open to the Unvanquished home page.

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

This is the way it looks with a 256×256 unvanquished icon as fallback instead of a 128×128 one:

unvanquished updater

@illwieckz illwieckz changed the title fetch full thumbnail instead of medium one, fix #39 a bunch of fixes, fix #39 #40 #42 Aug 20, 2019

@illwieckz illwieckz changed the title a bunch of fixes, fix #39 #40 #42 a bunch of fixes and improvements, fix #39 #40 #42 Aug 20, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 20, 2019

When we see this:

unvanquished updater

We notice user is more urged to browse the website than to download the game, and the instructions to browse the website is more obvious than to download the game or to play it.

This is a variant of the issue reported in #36 and tracked in #42.

I added obvious instructions on the progress bar, while I was at it I also increased a bit the progression info text size to make it more readable.

unvanquished updater

unvanquished updater

unvanquished updater

Notice the stealthy bottom notification is kept. This is not bad as it draws attention something happened:

unvanquished updater

unvanquished updater

We may also put the button on the left so the user naturally read it before reading text, making it more obvious, something he can't miss. The button would also be closer to the sentence talking about a “button”, then the user has less effort to make a cognitive association between those.

@illwieckz illwieckz force-pushed the fullthumbnail branch from dc7f20e to d164e86 Aug 20, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 20, 2019

So, I did this:

unvanquished updater

unvanquished updater

unvanquished updater

Note that I got the rounded-rectangle button shape by accident (height that is larger that the a bottom/top anchor), and liked it. I would like to know if it's expected or not…

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 20, 2019

@GamesOpenSource what do you think about the last three screenshots? 😃

@illwieckz illwieckz force-pushed the fullthumbnail branch 2 times, most recently from ad9fdc2 to b9e0a53 Aug 20, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 20, 2019

A little touch-up, I used a real symbol instead of a poor-looking > one in disappearing bottom notification:

unvanquished updater

Edit:, After some thoughts I just cut the notification after “Up to date”, there is no need to repeat how to play the game, and this is more coherent with other messages that just notice status but does not give instructions:

unvanquished updater

@illwieckz illwieckz force-pushed the fullthumbnail branch from c5f5b9f to edc5b32 Aug 20, 2019

News.qml Outdated Show resolved Hide resolved
News.qml Outdated Show resolved Hide resolved
@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2019

I found a better way to load the fallback posts json, it works anytime.

The code now logs the json url that is fetched:

qml: fetching posts json: http://www.unvanquished.net/index.php
qml: failed to parse posts json with error: SyntaxError
qml: message: JSON.parse: Parse error
qml: fetching fallback posts json
qml: fetching posts json: qrc:/resources/disconnected_posts.json
illwieckz added 4 commits Aug 18, 2019
large rework of news stuff, fallback thumbnail
- display the text even if the thumbnail is still loading,
  makes sure the news is readable and link clickable even
  if thumbnail fails to load
- use a fallback thumbnail if json does not provide it,
  use unvanquished icon
- always load full thumbnails,
  it looks far better
- use constant size for thumbnail and text zone
  it looks more coherent accross news
- crop thumbnail within updater
  required since thumbnail is displayed with custom size
- clicking on thumbnail also open news
  it's handy, and can be used as a fallback for link
- never overlap page indicator on thumbnail and text
- make page indicator visible over dark background and clickable,
  it works!
- resize title and body news font
  it fits more the text zone
- bold title font
- make grey body news font to make it more readable (less contrasted)
- add a subtle margin around text to make it more elegant
- clip the text (do not print outside of text zone)
also display fallback on thumbnail error
the rotating loading symbol is displayed until the
fallback is loaded or fails to load, then the thumbnail
is displayed or the fallback is printed instead

it is known to behave this way on wrong url, bad image
format, and network outage

@illwieckz illwieckz force-pushed the fullthumbnail branch from 7d80fea to 0b69aa6 Aug 22, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2019

This looks better with a 470×470 fallback icon 😃

unvanquished updater

See Unvanquished/Unvanquished#1118 about the story behind that icon 😉

@illwieckz illwieckz force-pushed the fullthumbnail branch from 0b69aa6 to 5155495 Aug 22, 2019

@Viech

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

Press the button to download the game.

Just "Continue reading." should be enough.

@illwieckz illwieckz force-pushed the fullthumbnail branch from 5155495 to 6699cc4 Aug 24, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

Press the button to download the game.

unvanquished updater

I find the . very weird, this is an instruction, this is text is not expected to be read as an informational content about the game. I think it's better to keep the sentence open since player is opening his mind to do something after that (playing the game), and the dot introduces a closing feeling which is the opposite we must look for. That's the same reason it's not a good things to end SMS or a tweet with a dot but it's better to keep things open, because it's open to what may happen after that.

It's also the same reason there is no ending dot in “stop” roadsign.

It looks to be a bad idea to add a dot to anything that is not a blog post nor text that is meant to be read as a story.

So at this point I added the the but not the dot:

unvanquished updater

@illwieckz illwieckz force-pushed the fullthumbnail branch from 6699cc4 to 1abfee2 Aug 24, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

Just "Continue reading." should be enough.

This part is done by Wordpress itself, this is why we also have no control on the color of the link (which is ugly). We use the excerpt Wordpress produces.

The other option would be to add updater ability to do the excerpt itself, by stripping all html markup from the complete article (which is already present in post json), truncate it, add an ellipsis then rebuild a link. This would be the best solution and it would give us absolute control on the excerpt length (we may truncate on character count instead of word count to avoid long words to overflow the text zone) and absolute control on link style (it would be better to reuse the cyan-ish color).

I'm already thinking about this but that's not for today and probably not for this PR.

@Viech

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

It's a proper sentence, you can hardly compare it to a stop sign. Sentences always end in a dot and it is always bad style if they don't. Even on Twitter, the 140 character limit does not make it good style to omit the dot, professional/official twitter feeds will use correct grammar. Coding style guides will force you to end your imperative comments with a dot even if they allow you like 70 characters tops (think of PEP 8 + PEP 257). Many minimalist advertisements will end their (imperative, all uppercase or all lowercase) sentences with a dot (think of "JUST DO IT.", "eat fresh.") regardless of how that makes them appear less exciting or how it could counteract the feeling of openness/freedom that they shall transport. We are no exception here.

@GamesOpenSource

This comment has been minimized.

Copy link

commented Aug 26, 2019

Kinda weird . discussion but I dig it.

I guess game launchers tend not to have full stops in short statements
image
image(short: no ., long: yes .)
image

I guess this one is somewhat comparable:
image

The play button should simply have "Play" in the button, like steam does, so you don't need to second-guess or read text to figure out how to play ^^:
like steam:
image
or epic:
image (well this is no play button because I have no epic games installed but still)
Or unity?
image

The statement could be simply "Unvanquished is up to date" or "Unvanquished ins ready to play"

Whatever, great job on fixing the issues!

PS: the blue link on black BG is unreadable by the way.

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 27, 2019

Yes, the “play” text in the button is not a bad idea, except the “download” button becomes a “play” one and those will not have the same time…

While I would tend to put dots everywhere because of purity, your examples illustrates wells the absence of dot in direct information (status, instructions) and the usage of dot in blablabla (explanation…). This looks to be the very common guideline out there.

@GamesOpenSource

This comment has been minimized.

Copy link

commented Aug 27, 2019

@GamesOpenSource what do you think about the last three screenshots? 😃

Oh sorry, I had missed that.

Great progress, glad it's being fixed!

When I run the updater now it immediately starts the game, not sure I can test anything until the updater gets a new release.

@DolceTriade

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

if you click the settings box in the splash screen, it will force open the updater and not open the game.

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 27, 2019

you would have to build the updater yourself to test those change, btw

illwieckz added 2 commits Aug 28, 2019
fix high-resolution updater icons
- fix bad pixels that look like a light rotation on png icon
- add border on png icon (copy border from unvanquished icon)
- redo the windows ico file based on high-resolution png icon

@illwieckz illwieckz force-pushed the fullthumbnail branch from f4fbb46 to 72dd3df Aug 28, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2019

@Viech I found a not-so-hard way to redo the link.

I stripped the original link from excerpt, then added a rectangle at the bottom (so it's always at the same place) with a brand new link we have full control on (text, style…).

While I was at it I clipped the excerpt in case it's so long it can reach the link. This may happen with very long words and/or user having custom large font size on his computer.

unvanquished updater

unvanquished updater

If we find a way to properly strip the html, we may just use the post itself and not rely on excerpt.

redo the post link
- strip the post link from excerpt
- redo link with proper style
- put link on bottom
- clip excerpt before the link

@illwieckz illwieckz force-pushed the fullthumbnail branch from 72dd3df to c2382cd Aug 28, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2019

I updated the link text (which is an instruction) to put a capital, to be consistent with other instructions:

unvanquished updater

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2019

I also redid the icon, using the icon by Gireen I found there:

unvanquished updater icon

Then I fixed some issues (small pixels were looking like there was a light rotation) and readded a border and a shasow by reusing the ones from the Unvanquished icon, I made the adjustments (repainting the reload symbol on border):

unvanquished updater icon

The Windows .ico file was redone with steps from 16 to 512.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.