Paths

Table of Contentst

Differential D689

Top Panel Tweaks
ClosedPublic
Actions

Authored by temple on Jun 28 2017, 5:46 AM.

Details

Reviewers

elexis

Commits

rP19843: Fix observermode GUI for 1024x768.

Summary

Here's screenshots of the changes to the top panel.

Make resources slightly closer together so that the population count doesn't overlap the civ icon in 1024 x 768 mode
Move the follow player button slightly closer to the civ icon
Remove the "Follow Player" text and replace it with a tooltip (the other icons have tooltips, so this seems natural)
Make the player dropdown relative width, so that it can be wider on wider screens (names won't take up multiple lines)
Reduce the dropdown's width in 1024 x 768 mode so it doesn't overlap with the follow player button

There is a bug where menu heights aren't always updated when the screen size changes, so you may notice that the height isn't correct when you switch from windowed mode to full-screen or vice versa. I think this affects all menus, but for fixed-width menus it doesn't matter because the new height is the same as the old height.

Test Plan

Test at different resolutions, see if the changes are reasonable.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

temple created this revision.Jun 28 2017, 5:46 AM

For 1024x768 the outcome is perfect and I agree the label should be hidden then.

But would it be possible to keep the label if there is enough space?
Also it seems unexpected that the checkbox is squeezed in beside the civ icon for big resolutions like 1920x1280 if there is tons of space in between.

Should be possible to use relative numbers that happen to work for all sizes, right?

resources.xml change seems ok (checked with max/cheat pop and max res). Doesn't really overlap for me on 1024x768 atm, but the change is ok.

elexis added a reviewer: elexis.Jun 28 2017, 12:33 PM

In D689#27498, @elexis wrote:

But would it be possible to keep the label if there is enough space?

Yes, I tried that first. It's messier (at least how I did it, maybe there's an easier way), and later I thought of the tooltip solution, which I liked a lot more. But I'll update the diff shortly with that change so you can see what you think. The top panel doesn't update when resizing the window, so sometimes the "Follow Player" label shows when it's not supposed to or vice versa. Updating top panel on tick solves this (as well as the dropdown height bug explained in the summary), but that seems overkill to me? I'm not an expert. We should only need to change it when either the player changes or the window resizes.

Also it seems unexpected that the checkbox is squeezed in beside the civ icon for big resolutions like 1920x1280 if there is tons of space in between.
Should be possible to use relative numbers that happen to work for all sizes, right?

I think my ideal solution would be have it as a tooltip and keep the checkbox just to the left of the dropdown instead of to the right of the civ icon. It's intuitive for it to stay connected the player dropdown. That's an easy change.

resources.xml change seems ok (checked with max/cheat pop and max res). Doesn't really overlap for me on 1024x768 atm, but the change is ok.

This is what I see. (Could you post a screenshot if yours looks different than mine?) I never play on 1024 x 768, so it doesn't matter much to me. I guess I should better position the "Alpha 22" too.

Here the "Follow Player" label is hidden if it overlaps with the player dropdown. (Just for testing, not a final version.)

In D689#27516, @temple wrote:

Updating top panel on tick solves this (as well as the dropdown height bug explained in the summary),

Oh, really. It's the viewPlayer.hidden update that corrects the wrong dropdownlist size?

but that seems overkill to me?

Yes, should be avoided to do things ontick.
We could add a JS notification if the window is resized, could be done in a separate diff. A ScriptEvent call would have to be done on SDL_WINDOWEVENT_RESIZED or SDL_WINDOWEVENT_SIZE_CHANGED somewhere in source/gui/ (https://wiki.libsdl.org/SDL_WindowEventID).
(The dropdown bug should still be fixed properly)

It doesn't look like the caption/tooltip change would cause a relevant *pot, so it probably doesn't break alpha 22 string freeze, should check twice.

I think my ideal solution would be have it as a tooltip and keep the checkbox just to the left of the dropdown instead of to the right of the civ icon. It's intuitive for it to stay connected the player dropdown. That's an easy change.

There is a Engine.GetTextWidth(font, text) function, for example used in network.js.

(We keep in mind that there is still a plan to add more options to the observermode (ticket somewhere) and thus move the options somewhere else (for example similar to the gamespeeds dropdown that pops up if clicked).)
However it still seems cleaner to have the label shown, even if that's not easy, no? It's weird to have a checkbox without label.
Let me see if I can add the resize notification, if that makes it possible to keep the label up there.

This is what I see

You're right, I didn't build enough houses and population.

Hid the "Follow Player" label when covered by the player dropdown (the threshold is around 1260 width).
Kept the follow player tooltip (at all widths).
Kept the resource width adjustment.
Adjusted the alpha label size to better match the available space in both playing and observer mode. (It's possible to get the size from the other objects rather than hard-code it in session.js, but that seemed like too much hassle.)

I assumed the follow player tooltip would be okay in translations because it's the same words and context as the follow player label, but I don't know how that all works so someone else will have to check.

elexis mentioned this in D692: Fix Dropdown height after resizing.Jun 29 2017, 3:36 AM

elexis mentioned this in D694: Allow the JS GUI to subscribe to window resize events.Jun 29 2017, 4:06 AM

If D692 is accepted and if

	<action on="WindowResized">
		updateTopPanel();
	</action>

is added to session.xml, then the label will be updated correctly without abusing onTick

Thanks, that should be everything for now :-) Those two GUI engine bugs should be fixed next.

It definitely doesn't break string freeze, confirmed by running maintenanceTasks.sh:

Index: public-gui-ingame.pot
===================================================================
--- public-gui-ingame.pot	(revision 19842)
+++ public-gui-ingame.pot	(working copy)
@@ -1383,25 +1383,26 @@
 msgid "Objectives"
 msgstr ""
 
-#: gui/session/session.xml:(caption):77
+#: gui/session/session.xml:(caption):76
 msgid "Game Paused"
 msgstr ""
 
-#: gui/session/session.xml:(caption):80
+#: gui/session/session.xml:(caption):79
 msgid "Click to Resume Game"
 msgstr ""
 
-#: gui/session/top_panel.xml:(caption):22
+#: gui/session/top_panel.xml:(tooltip):23
+#: gui/session/top_panel.xml:(caption):29
 msgctxt "observer mode"
 msgid "Follow Player"
 msgstr ""
 
-#: gui/session/top_panel.xml:(caption):47
-msgid "Observer Mode"
+#: gui/session/top_panel.xml:(tooltip):46
+msgid "Choose player to view"
 msgstr ""
 
-#: gui/session/top_panel.xml:(tooltip):39
-msgid "Choose player to view"
+#: gui/session/top_panel.xml:(caption):54
+msgid "Observer Mode"
 msgstr ""
 
 #: gui/session/trade_window.xml:(caption):9

Another thing for dropdowns in general I noticed is that if the selected player text requies multiple lines, something like 5 pixels of the second line are rendered in the dropdown itself (not the selection list). That should be fixed too, exactly the number of lines that completely fit in there should be rendered.

This revision is now accepted and ready to land.Jun 29 2017, 7:12 PM

Closed by commit rP19843: Fix observermode GUI for 1024x768. (authored by elexis). · Explain WhyJun 29 2017, 7:26 PM

This revision was automatically updated to reflect the committed changes.

elexis mentioned this in rP19846: Allow the JS GUI to subscribe to the window resized event and use it to….Jun 30 2017, 2:04 AM