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

Take screenshot in dialogs #13184 #13186

Merged
merged 1 commit into from Apr 30, 2017

Conversation

Projects
None yet
3 participants
@rob-v
Contributor

rob-v commented Apr 24, 2017

Closes #13184
Closes #9508
Take screenshot works fine ingame or in main menu, doesn't work usually in lobby and all dialogs.
I was thinking if this is needed/worth, but the issue is that:

  1. Take screenshot works e.g. in lobby or any tab/dialog until you activate Text field. Then it is not possible to take screenshot without moving to another tab/window and back (clicking another widget like checkbox on the same tab doesn't help). So it works at beginning but then stops to work so obviously looks like/is bug.
  2. You can take screenshot from Windows even while in Textbox

This small change enables screenshot also in lobby/dialogs anytime.
Note: it is small code duplication. it could be done only once in Ui.HandleKeyPress with little perf impact.

@pchote

This seems like the most reasonable solution we can do, and IMO is an improvement over the hardcoded RootWidget class.

Can you please move over the rest of the RootWidget keys, and then replace that with a normal widget?

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Apr 26, 2017

Contributor

Updated.
I was thinking about replacing Down by Up or Down with MultiTapCount (#13194) for these rare/longer operations.

Contributor

rob-v commented Apr 26, 2017

Updated.
I was thinking about replacing Down by Up or Down with MultiTapCount (#13194) for these rare/longer operations.

@pchote

pchote approved these changes Apr 30, 2017

LGTM. 👍 after the following fix

Show outdated Hide outdated OpenRA.Game/Widgets/Widget.cs

@pchote pchote added the PR: Needs +2 label Apr 30, 2017

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Apr 30, 2017

Contributor

Updated.

Contributor

rob-v commented Apr 30, 2017

Updated.

@reaperrr reaperrr merged commit d5740ef into OpenRA:bleed Apr 30, 2017

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr
Contributor

reaperrr commented Apr 30, 2017

@rob-v rob-v deleted the rob-v:TakeScreenshotInDialogs branch Apr 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment