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

Improve the layout of the "load game" page #1166

Merged
merged 7 commits into from
Aug 2, 2022

Conversation

lmoureaux
Copy link
Contributor

This PR changes the layout of the "load game" screen to jump around less. There are still huge improvements to be made but I think it's a good start.

The new layout looks like this:
image

After #1165.
Related to #115.

@jwrober
Copy link
Collaborator

jwrober commented Jul 30, 2022

So with the default NightStalker theme, the dialog overlaps the LongTurn logo as shown above and here.

image
image

Looks unprofessional. I would suggest increasing the size the the dialog so the image shows uncovered in the lower right corner. Since the other shipped themes don't have an image there it should be fine.

Also if you open the dialog, click cancel and then load it again you will briefly see the new game dialog and then it is replaced by the load game dialog.

Lastly, if you open the dialog, pick a saved game to show the preview, click cancel and then load saved game again, the preview of the game you clicked on the first try still shows, but the save game file in the table is not selected. I think it should be clean every time.

@lmoureaux lmoureaux marked this pull request as ready for review July 30, 2022 19:06
@lmoureaux
Copy link
Contributor Author

Also if you open the dialog, click cancel and then load it again you will briefly see the new game dialog and then it is replaced by the load game dialog.

I don't think this one is new.

@jwrober
Copy link
Collaborator

jwrober commented Jul 30, 2022

Also if you open the dialog, click cancel and then load it again you will briefly see the new game dialog and then it is replaced by the load game dialog.

I don't think this one is new.

I can recreate in beta.3, so yea not new. But if can be fixed here while the code is being worked on would be good.

@lmoureaux lmoureaux added the gui This issue requires changes to the user interface label Jul 30, 2022
@lmoureaux
Copy link
Contributor Author

Also if you open the dialog, click cancel and then load it again you will briefly see the new game dialog and then it is replaced by the load game dialog.

I don't think this one is new.

I can recreate in beta.3, so yea not new. But if can be fixed here while the code is being worked on would be good.

Looks like a timing-dependent issue, it doesn't happen very often for me.

lmoureaux added a commit to lmoureaux/freeciv21 that referenced this pull request Jul 30, 2022
The column with game information in the page_load dialog (to the right of the
window) was still jumping around depending on the size of the map. There were
also concerns that the Longturn logo shown at the bottom right with the
Nightstalker theme was hidden (which still happens when the window is small,
but fixing that is a bigger change).

The column size is hacked in by having the right column expand to 25% of the
window width.

See longturn#1166.
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this pull request Jul 30, 2022
When loading the preview fails because we couldn't extract the preview from the
save, be conservative and disable the "Load" button (how could the server load
the file if we can't?). Also hide the preview from the previously selected
item, if any.

See longturn#1166.
@lmoureaux
Copy link
Contributor Author

Should have addressed most concerns. The LT logo will still be partially covered at small window sizes, but now it should at least be fine when the window is big enough.

Copy link
Collaborator

@jwrober jwrober left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@lmoureaux
Copy link
Contributor Author

OS X CI: https://gitlab.freedesktop.org/ is down

Copy link
Contributor

@psampathkumar psampathkumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Can merge after rebasing.

Less jumping around = better UX.
This way the buttons are in the correct (platform-dependent) location and get
assigned the right icon automatically.
The user doesn't need to know the time zone. It uses their current local time
anyway.
The column with game information in the page_load dialog (to the right of the
window) was still jumping around depending on the size of the map. There were
also concerns that the Longturn logo shown at the bottom right with the
Nightstalker theme was hidden (which still happens when the window is small,
but fixing that is a bigger change).

The column size is hacked in by having the right column expand to 25% of the
window width.

See longturn#1166.
Select the latest save when we switch to page_load. This way, to continue the
latest game one can just click on "Load".
When loading the preview fails because we couldn't extract the preview from the
save, be conservative and disable the "Load" button (how could the server load
the file if we can't?). Also hide the preview from the previously selected
item, if any.

See longturn#1166.
@lmoureaux lmoureaux enabled auto-merge (rebase) August 2, 2022 12:29
@lmoureaux lmoureaux merged commit 19d3d53 into longturn:master Aug 2, 2022
@lmoureaux lmoureaux deleted the ui/page-load branch August 2, 2022 12:43
lmoureaux added a commit that referenced this pull request Aug 2, 2022
The column with game information in the page_load dialog (to the right of the
window) was still jumping around depending on the size of the map. There were
also concerns that the Longturn logo shown at the bottom right with the
Nightstalker theme was hidden (which still happens when the window is small,
but fixing that is a bigger change).

The column size is hacked in by having the right column expand to 25% of the
window width.

See #1166.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gui This issue requires changes to the user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants