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

Improved city screen queue #12737

Merged
merged 12 commits into from
Jan 4, 2025
Merged

Conversation

sulai
Copy link
Contributor

@sulai sulai commented Jan 2, 2025

See previous PR #12341.

Improved since then:

  • queue preview entries selectable
  • can be removed from queue while selected as preview

mrimvo and others added 7 commits October 19, 2024 18:54
- moved "buy" button to city info table
- removed "add to queue" button
- expand icon changed to android defaults
- CityStatsTable: big scrollable area, expandable
…cramped portrait

- moved priority buttons under the queue
- queue is collapsible and shows preview
- when expanded, the queue takes available space
- collapse stats on cramped portrait
- collapsed stats show 2 lines of resources in cramped portrait to reduce width, right aligned
- raze button moved close to city button in cramped portrait
…-Queue

# Conflicts:
#	core/src/com/unciv/ui/components/widgets/ExpanderTab.kt
#	core/src/com/unciv/ui/screens/cityscreen/BuyButtonFactory.kt
#	core/src/com/unciv/ui/screens/cityscreen/CityConstructionsTable.kt
#	core/src/com/unciv/ui/screens/cityscreen/CityStatsTable.kt
@NahuelGeno
Copy link
Contributor

This improvement to the city building queue is really needed, I'm looking forward to it. The previous PR you mentioned hasn't been implemented yet, right?

Copy link
Owner

@yairm210 yairm210 left a comment

Choose a reason for hiding this comment

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

I'm not entirely sure but the preview is definitely good, let's make the change and see what people like / dislike

@sulai
Copy link
Contributor Author

sulai commented Jan 2, 2025

Thank you guys for your positive feedback! 🙂 Some of these changes are 2 months old, I'll double check those tomorrow 👍

@sulai
Copy link
Contributor Author

sulai commented Jan 3, 2025

I double-checked all changes and I think we now have a decent state. The only thing that bothers me a little is that hack with queueExpander.toggle(), but I don't know any better way of doing it. It works from the user's perspective, but it's a hack.

@NahuelGeno to answer your question, the previous PR wasn't merged because it "timed out". I was lacking time to finish it.

@yairm210 I agree we should listen to the community for further improvements. By introducing the mini-queue, this PR focuses on cramped portrait and cramped landscape android screens. That is, the average android phone people likely have. I added some in-code reasoning as to how it manages vertical space better (Landscape). It also handles horizontal space better in cramped-portrait by moving the priority buttons. There's still room for improvement though since some entry descriptions tend to waste some horizontal space, but let's postpone that.

From my perspective, (if we don't find a better way for that toggle-hack), the PR is ready to merge 😃 🎉

@yairm210 yairm210 merged commit 9ce439f into yairm210:master Jan 4, 2025
4 checks passed
@sulai sulai deleted the improved-CityScreen-Queue branch January 5, 2025 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants