-
Notifications
You must be signed in to change notification settings - Fork 41
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
Initial implementation of the top bar #951
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lmoureaux
added
enhancement
New feature or request
refactoring
This issue requires code refactoring
gui
This issue requires changes to the user interface
labels
Mar 20, 2022
lmoureaux
force-pushed
the
feature/top-bar
branch
from
March 20, 2022 02:35
5cba9b0
to
848d4e6
Compare
Looks good to me, just a couple of conflcts due to the recent NULL -> nullptr in master. I say, we merge it and then tweak the topbar as we go along. This is a good start. |
We decided that the contents of the sidebar would be shown above the map. Start this change by moving the widget to the top. See longturn#940.
QToolButton seems a better fit because it already provides most of the options we need. Maybe the top bar should become a fully-fledged QToolBar. See longturn#940.
Simplifies the implementation. See longturn#940.
Replace custom-drawn icons with a standard QToolButton feature. Now they're even SVG-capable! See longturn#940.
This simplifies the code a little. See longturn#940.
We'll rethink the layout later; for now, simplify it. See longturn#940.
Simplifies the code. See longturn#940.
It was used for a tiny graphical tweak of little importance. See longturn#940.
Themes can use QSS if they want something special. See longturn#940.
It's not perfect but we're not after perfection. It works. See longturn#940.
It wasn't used, removing it simplifies the code. See longturn#940.
I'm not sure what this will become so I'm implementing an ugly placeholder that does the job. We may want to implement the switching with a set of mutually exclusive QAction. See longturn#940.
Some of them can only be clicked (e.g. End Turn). See longturn#940.
Use a specialized class instead of a special mode of top_bar_widget. See longturn#940.
Use a near-copy of tax_rates_widget instead of a special mode of top_bar_widget. See longturn#940.
Code cleanup after moving the functionality to separate classes. See longturn#940.
lmoureaux
force-pushed
the
feature/top-bar
branch
from
March 25, 2022 21:21
848d4e6
to
9d4533b
Compare
Rebased. |
I think the only thing this needs now is the client themes to support dark mode as I mentioned in Discord. |
The code was previously saying that it didn't want it to be drawn, leading to artifacts with semitransparent buttons. Now the area is always cleared before painting. See longturn#951.
They were fully transparent.
Theming done. |
jwrober
approved these changes
Mar 26, 2022
jwrober
pushed a commit
that referenced
this pull request
Mar 27, 2022
jwrober
pushed a commit
that referenced
this pull request
Mar 27, 2022
The code was previously saying that it didn't want it to be drawn, leading to artifacts with semitransparent buttons. Now the area is always cleared before painting. See #951.
jwrober
pushed a commit
that referenced
this pull request
Mar 27, 2022
jwrober
pushed a commit
that referenced
this pull request
Mar 27, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
New feature or request
gui
This issue requires changes to the user interface
refactoring
This issue requires code refactoring
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR turns the side bar into a top bar. I did some refactoring and simplification of the side/top bar code along the way, which explains the source lines count.
This is intended as a first step so we can start creating issues about specific features. Bugs are expected and I'll only solve very easy ones as part of this PR. Theme support is left for later (currently, the widgets use the system theme).
Only tested with Nightstalker and a dark system theme (Breeze Dark to be specific).
Closes #940.
(Or if we consider 940 as a meta issue, closes the first child issue.)