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

New MainMenu which has smart button layout, scaling and all that fancy thing #819

Merged
merged 14 commits into from
Apr 17, 2020

Conversation

kvakvs
Copy link
Collaborator

@kvakvs kvakvs commented Apr 7, 2020

Fixes: #410
Fixes: #437

Replace old style U UI for MainMenu with new smarter UI style.

bild

https://user-images.githubusercontent.com/288724/78613640-a8157100-786c-11ea-9e02-a85f3765c976.png
bild

One minor annoying thing, recreating Main Menu on options changes also closes it. Otherwise worked clean for me during multiple hours of tests and multiple reloads.

TODO

  • Delete string (Menu) "Tooltip.Keybinds:Change lane arrows"
  • Remove keybinds from the Lane Arrows tooltip
  • Make + in Modifier+Click white
  • Hook on Windows transparency Options slider and try make it work
  • Drag handle doesn't get updated on UI rescaling and forces the window to be large (suggestion: Make an autosizing option to fit the parent?)
  • With 5 tool buttons split rows as 3+2 not as 4+1
  • On Options.RebuildMenu the mainmenu toggle button sprite is wrong
  • A new ? button to toggle keybinds panel
  • Rename Despawning tooltip: add more info
  • Capitalize tool tooltips
  • Low transparency requires special handling of the gray panel
  • Toggling features in Maintenance rebuilds the mainmenu and that resets its transparency
  • Update Despawn/Clear? button tooltip on click.
  • Check parking tool active / normal texture

WIP: Menu buttons wrapping logic

Main menu grand rework; Reload fixed; Extra info labels hanging outside

Experimental: Mouse in UI test
@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 7, 2020

One thing to discuss:
Where to place the button to open and to close the keybinds panel.

  • It can be a button to open and close, then it will take space in the main menu panel.
  • Or it can be a close button in the keybinds panel, then its easy to close, but not sure how to open again (of course via the options panel but that's well hidden away, many will not go there and not re-enable it).
    bild

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 10, 2020

Based on UI state of the Lane Arrow tool the following OSD shortcuts are shown:
Select mode: Shows Ctrl+Click (localized) and Alt+Click (localized)
Edit mode: Shows "Delete|Bkspace Reset to default"

bild

bild

P.S. Spent sizeable amount of time fighting destroyed controls which have not yet left the hierarchy.

@kianzarrin
Copy link
Collaborator

kianzarrin commented Apr 10, 2020

Cool stuff!

I set my lane arrow tool keybind to CTRL+ALT+SHIFT for testing.
Screenshot (801)

This is what I got:
Screenshot (804)
Screenshot (802)

The tool-tip should only show the shortcut key (if any).

EDIT: It would be nice if the "+" between the hotkeys was white but still maintain the black highlight like this:
Untitled

EDIT: is it possible to change the color of the tool hotkey text in tooltip text so that they are consistent brown color.

@kianzarrin
Copy link
Collaborator

The width of the Main menu window/panel does not get updated as I resize scale.

Screenshot (805)

Copy link
Collaborator

@kianzarrin kianzarrin left a comment

Choose a reason for hiding this comment

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

see comments above. FYI I edited one of them

"General.Dropdown.Option:Medium","Mittel","Medium","Medio","Moyenne","Közepes","Medio","中くらい","중간","Middelhoog","Średnia","Média","Среднее","高","中","Medium","Orta","Середня"
"General.Dropdown.Option:High","Hoch","High","Alto","Haute","Magas","Alto","高い","높음","Hoog","Wysoka","Alta","Высокое","高","高","High","Yüksek","Висока"
"General.Dropdown.Option:Very high","Sehr hoch","Very high","Muy alto","Très haute","Nagyon magas","Molto alto","とても高い","매우 높음","Zeer hoog","Bardzo wysoka","Muito Alta","Очень высоко","非常高","非常高","Very high","En yüksek","Дуже висока"
"Shortcut.Modifier:Shift","","Shift","","","","","","","","","","","","","Shift","",""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity why is it necessary to translate modifier keys?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are only translated for game-supported languages using the game code, and only while they are a part of a InputKey or SavedInputKey. We have more languages than the game. So at least modifier keys should be translated.

@kianzarrin
Copy link
Collaborator

kianzarrin commented Apr 10, 2020

I don't know if it is related to this PR but Changing window transparency does not work at all:
Screenshot (810)
Screenshot (809)

@kianzarrin kianzarrin added the UI User interface updates label Apr 10, 2020
@kianzarrin
Copy link
Collaborator

There is opportunity to make panel smaller horizontally when removing some features:
Screenshot (813)
Screenshot (812)

@kianzarrin
Copy link
Collaborator

I hope this is not too picky but this is weird and unexpected:
If I only change one TMPE tool. TMPE button will be in a strange state where it is pressed but the panel is not visible. to see the panel I have to click once to disable TMPE and another time to enable TMPE.
If I change two or more TMPE tools then TMPE will be completely disabled.

In the picture bellow:

  1. Go to options while TMPE panle is open (this is actually hard to reach because must users use ESC to go to options which closes TMPE. I pressed the option buttong to do so).
  2. Activate/Deactivate only one tool. -> TMPE button is blue and in active state but the panel has disappeared. is TMPE active at this point?
    3- close options. click on TMPE button -> the button will go into grey inactive state.
    4- click on TMPE button twice -> TMPE panel will open.

Untitled2

@originalfoo originalfoo added the Usability Make mod easier to use label Apr 11, 2020
@originalfoo originalfoo added this to the 11.4.0 milestone Apr 11, 2020
@krzychu124
Copy link
Member

Regards to keybinds panel show/hide toggle, maybe you could position it there:

image

That way we wouldn't need to widen whole main menu panel just for that one small button?

@originalfoo
Copy link
Member

originalfoo commented Apr 11, 2020

Regards to keybinds panel show/hide toggle, maybe you could position it there:

What happens when the toolbar panel shrinks if features are disabled? The TMPE text label would cover it.

Regarding toggle button, maybe a small ? would do? User hovers it and sees tooltip explaining what it does, clicking it toggles the shortcut hints panel.


I found this tooltip confusing:

image

Could we change to something like:

Toggle vehicle despawning
Currently state

And the state would be either:

  • enabled (easy mode)
  • disabled (hard mode)

This way it is also giving user some hint that disabling despawning will make gameplay harder. The current state line could maybe also be different color (green or red?) to clarify that it's changeable part of tooltip?

I'll do more detailed review later, as I have to check Kian's mass edit stuff first.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 11, 2020

The despawning text change belongs to Crowdin.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 12, 2020

@aubergine10 the "Normal" textures are indeed 50% desaturated compared to "Active" textures.

@originalfoo
Copy link
Member

So is the issue that the others are too bright in normal state?

@kianzarrin
Copy link
Collaborator

Mode-specific options below?

maybe only show an icon per options group and if someone presses it then mod specific options appear. I don't know if that would be within the scope of this review.

@kianzarrin
Copy link
Collaborator

Window transparency is working :) but the higher the transparency value, the less transparent it is. what we are really changing here is opacity.

Copy link
Collaborator

@kianzarrin kianzarrin left a comment

Choose a reason for hiding this comment

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

This is looking awesome!
are all previous issues have been fixed.
new issue: see comments about transparency.

One question you forgot to answer: are you able to change tooltip text to highlight keybinds?

TLM/TLM/U/Panel/BaseUWindowPanel.cs Outdated Show resolved Hide resolved
@kianzarrin
Copy link
Collaborator

Regards to keybinds panel show/hide toggle, maybe you could position it there:
image
That way we wouldn't need to widen whole main menu panel just for that one small button?

Its better to be on the top left. so that with different languages there is no chance of the text covering the button.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 12, 2020

The new button change and the new options to show/hide the keybinds hint panel is already almost done, i am debugging possible problems.
The button is hooked on the right of the version label, and as the version label is usually shorter than the parent form, it doesn't look perfect. I might later add a new layout feature like align to the parent's right edge, and then it'll look good, but i don't want to infinitely grow the scope of this ticket.

@originalfoo
Copy link
Member

Good catch on transparency vs. opacity. I felt the slider was working backwards but hand't given it too much thought.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Apr 12, 2020

Made GlobalConfig.Menu.GuiTransparency obsolete and added new GuiOpacity. Its rather that, than converting the inverse value or trying to migrate with unclear results and keeping the mess.
Strings might need to be updated, and string keys. But for example in Russian opacity uses same word as transparency with inverting prefix, so kind of interchangeable.

bild

Copy link
Member

@krzychu124 krzychu124 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, no issues found yet

@kvakvs kvakvs merged commit 07332b0 into CitiesSkylinesMods:master Apr 17, 2020
@kianzarrin
Copy link
Collaborator

A few peaky issues that I don't consider to be a blocker:
1- Its possible to do this :
Screenshot (850)
I made the panel small -> moved it to button of screen -> made it big.

2- dragging can only happen if you drag the label:
Screenshot (849)

@originalfoo originalfoo added the Keybinds Keyboard (and mouse) shortcuts label Apr 17, 2020
@originalfoo originalfoo linked an issue Apr 17, 2020 that may be closed by this pull request
ModUI.GetTrafficManagerTool().RequestOnscreenDisplayUpdate();

// The task is delayed till next GUI update frame.
// ModUI.GetTrafficManagerTool().InvalidateOnscreenDisplayFlag = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commented out code without TODO

lastUpdatePositionFrame_ = nowFrame;
}

// if (isStarted_) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commented out code without TODO

@originalfoo originalfoo added the Localisation Localised text and features label May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Keybinds Keyboard (and mouse) shortcuts Localisation Localised text and features Toolbar The main TMPE toolbar UI User interface updates Usability Make mod easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Menu bar buttons escape the menu bar Move Keybind code to UI namespace On-screen display mod
4 participants