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

Fix the chat widget #1265

Merged
merged 11 commits into from
Aug 16, 2022
Merged

Conversation

lmoureaux
Copy link
Contributor

@lmoureaux lmoureaux commented Aug 13, 2022

Had some "fun" reviewing the chat widget code and fixing most open issues. See commit messages for details.

Closes #1056.
Closes #1074.
Closes #1145.
Closes #1144.
Closes #1143.
Closes #1159.
Closes #1158.
Closes #739.

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.

  • 1074 is resolved
  • 1145 is resolved - really enhances the ease of resizing both chat and messages!
  • 1144 is resolved
  • 1143 is resolved - however it is really hard to grab the upper right corner with the minimize button the way it is right now. Maybe move the minimize button to the left a little bit?
  • 1159 is resolved - chat now is below messages and above the minimap
  • 1158 is partially resolved. If you move the chat widget to a spot on the screen it does stay there while in the game, however the location is not saved so the location is not the same when you come out and come back in to a game. I think it should stay where I put it until I put it in another spot.
  • 739 is resolved

One other issue I noticed is you can grab the title bar of the messages box and drag it down. I think it needs to be pinned to the top bar. The new Qt::Edge deal is great otherwise.

@lmoureaux
Copy link
Contributor Author

If you move the chat widget to a spot on the screen it does stay there while in the game, however the location is not saved so the location is not the same when you come out and come back in to a game. I think it should stay where I put it until I put it in another spot.

Works for me

lmoureaux added a commit to lmoureaux/freeciv21 that referenced this pull request Aug 14, 2022
When hovering widgets in the chat box that also happen to be within the
resizing area of resizable, the cursor was being set to the resize arrow even
though clicking wasn't triggering a resize. Set the cursor explicitly on these
widgets to make it less confusing.

See longturn#1265.
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this pull request Aug 14, 2022
@lmoureaux
Copy link
Contributor Author

it is really hard to grab the upper right corner with the minimize button the way it is right now

The resize arrow cursor is now only shown when resizing would work

you can grab the title bar of the messages box and drag it down

My bad, fixed

lmoureaux added a commit to lmoureaux/freeciv21 that referenced this pull request Aug 14, 2022
When hovering widgets in the chat box that also happen to be within the
resizing area of resizable, the cursor was being set to the resize arrow even
though clicking wasn't triggering a resize. Set the cursor explicitly on these
widgets to make it less confusing.

See longturn#1265.
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this pull request Aug 14, 2022
@lmoureaux lmoureaux force-pushed the bugfix/chat-move-resize branch from 29fe901 to becec8b Compare August 14, 2022 18:24
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this pull request Aug 14, 2022
The size of the minimap has been fixed for some time now. Old settings could
still interfere with the location of the chat widget, so remove any code
touching those.

See longturn#1265.
@lmoureaux
Copy link
Contributor Author

If you move the chat widget to a spot on the screen it does stay there while in the game, however the location is not saved so the location is not the same when you come out and come back in to a game. I think it should stay where I put it until I put it in another spot.

New commit addressing this after our discussion and reproducing it on my side.

The chat widget had acquired a minimum size in
f29091e, which prevented it from minimizing
correctly. Change the minimum size to fix minimization, and prevent resizing
the widget while it is minimized.
Minimizing the chat widget should never send it out of screen. Make sure that
it sticks to the side if that would happen.

Closes longturn#1074.
Instead of letting the user of the class allow resizing of each corner
separately, only use the edges to specify which resizes are allowed. Usable
corners are deduced from the list of edges. This makes for a more concise
implementation and lets us use Qt::Edges instead of a custom enum.

Closes longturn#1145.
The few extra pixels make grabbing the edges a lot easier, greatly improving
usability.
setResizable({}) is just as easy.
This arithmetic is complicated and I'm not sure how to get it right. Use a
simpler algorithm: try to put the edge where it should be, and move it back if
the resulting height or width would become smaller than the minimum.

Closes longturn#1143.
This way it looks more like a popdown.

Closes longturn#1159.
The geometry of the chat_widget (relative to the game page) is stored to disk.
The values are also used when resizing the widget and when expanding it, making
everything quite complicated. Keep the same design but make sure that the
values to be saved are always up-to-date.

Closes longturn#1158.
That's what we use when restoring it.
Setting the minimum size was making the widget taller and changing bottom(),
breaking geometry computations afterwards. Save the geometry before calling
setMinimumSize().
@lmoureaux lmoureaux force-pushed the bugfix/chat-move-resize branch from 8cb5f00 to f5b9015 Compare August 15, 2022 23:51
@lmoureaux lmoureaux enabled auto-merge (rebase) August 15, 2022 23:52
@lmoureaux lmoureaux merged commit 68b5063 into longturn:master Aug 16, 2022
@lmoureaux lmoureaux deleted the bugfix/chat-move-resize branch August 16, 2022 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment