Add screenshot button to power menu

[kkeijzer]

This small patch adds a screenshot button to the power menu.

This is particularly welcome for the PinePhone, where the volume buttons cannot register both a VolUp and VolDown keypress at the same time.

I have also heard that other mobile OS'es add this button in this place in addition to a key combination. This would add it to Ubuntu Touch as well.

[cibersheep]

Mhm... I understand the goal but it doesn't make much sense to me to have a screenshot button in the Power Down menu to be honest.

Maybe in the Session indicator would be a better place?

[kkeijzer]

I agree with you that it's weird. I actually asked this very question myself in the Ubuntu Touch on PINE64 channel earlier.

But then some of the more senior developers like @UniversalSuperBox, @dobey and @kugiigi seemed to be in favour of doing it this way, so that's why I gave it a shot.

[kugiigi]

Some android phones have this as well in the power menu so I guess it's fine. Hardware button combo is still the better way to fo screenshots but it's good to have a software-only solution. Puttimg it in the indicator would be a lot more complicated in my guess.

[cibersheep]

• Sorry I missed that message in the channel
• It sounds like the old «turn off going to start» paradigm
• I think we should really stop looking at what other OS do.
• How is it done in Ubuntu (if not via keyboard)? Via an app, right?

[kkeijzer]

Yes, there's gnome-screenshot normally, or command line tools like scrot, or third party apps like Shutter. And then there's also GNOME Shell extensions like this.

[UniversalSuperBox]

There are multiple cases for it:

• Temporarily, we have no solution for screenshots on the PinePhone. Both volume keys can't be pressed at once and it is annoying.
• More permanently, this provides a small accessibility boost for people with limited motion. Pressing two buttons at once, especially if they are on opposite sides of the device, is difficult for some people. That's why we try to avoid making things multi-touch only and provide single-touch buttons for many gestures.

However, Joan is correct and I didn't consider it before. The Session indicator would be another logical place to put this, and would solve both use cases: https://github.com/ubports/indicator-session

It would be more difficult for sure.

[PhoenixLandPirate]

The Session indicator would be another logical place to put this

This seems fine on a phone which you can screenshot when using hardware buttons, however, what if on the pinephone I want to take a screenshot of the notification indicator or the audio indicator?

The biggest problem with none-hardware based screenshot methods, is the speed issue, if you need or want to take a screenshot of a notification bubble, or of a chat that's moving quickly, or something else that's moving quickly, however I don't think thats fixable without being annoying. Another issue is where ever you put it, it seems like you might miss something,

I think Joan is right when talking about screenshot apps, because with an app you can add fun extras like

• "Screen shot in 5 seconds when I'm ready" or "Screenshot the next time I press the volume down button",
• you could find a way to add select regions like you can on KDE.
• You could add the scroll down the app, take several screenshots and stitch them together, like in android.
• Have the screenshot just be shared, or copied to clipboard without it being saved and added to the gallery app.

I think maybe hardware screenshots could be instant, than open the app and ask what you want to do with it, and the screenshot via indicator or app could be tweaked a bit more before the screenshot or something, maybe the separation doesn't really have to happen, maybe both options could just take a screenshot than send you to the app, and ask if you want to retake it but in 5 seconds, or if you want to scroll down the app and stitch, or keep with the current screenshot, and if you want to keep with the current, do you want to save it or share it?

[kugiigi]

A dedicated app would definitely be better but I feel like that's too advanced already.
The main point of this implementation is to put a simple way to take a screenshot without hardware buttons.
The proper and simplest approach in my mind is to put an item in the session indicator as already mentioned. This will then open a dialog similar to the shut down dialog where you can select a few options like take a screenshot now, take a screenshot after closing the dialog, or after a few seconds. The problem is I'm not sure if it's possible to close the indicators from a code.

[cibersheep]

Checking on how «Report a bug» is working, an entry in the indicator that opens a link we have:

The «Report a bug» entry is here:
https://github.com/ubports/indicator-session/blob/xenial/src/service.c#L466

Its definition here(?):
https://github.com/ubports/indicator-session/blob/xenial/src/actions.h#L79
https://github.com/ubports/indicator-session/blob/xenial/src/service.c#L1050

Its action is here:
https://github.com/ubports/indicator-session/blob/xenial/src/service.c#L951
https://github.com/ubports/indicator-session/blob/xenial/src/actions.c#L293
https://github.com/ubports/indicator-session/blob/xenial/src/backend-dbus/actions.c#L1078
here the indicator is closed ¿because focus off?:
https://github.com/ubports/indicator-session/blob/xenial/src/backend-dbus/actions.c#L841

[UniversalSuperBox]

what if on the pinephone I want to take a screenshot of the notification indicator or the audio indicator

Indeed, or if you have trouble reaching multiple buttons at once and want to take a screenshot of an indicator.

Well, that brings us back around to the button in the power menu. It allows screenshots to be taken of everything but the Power menu.

If we go forward with this, we would need to reword the Power dialog so it includes powering off or taking a screenshot. It would make the dialog go against our HIG. Maybe that is worth it in the name of accessibility?

[cibersheep]

what if on the pinephone I want to take a screenshot of the notification indicator or the audio indicator

Indeed, or if you have trouble reaching multiple buttons at once and want to take a screenshot of an indicator.

I think in this case, the «app» to take screenshots in x seconds will be handful. So you can take screenshots of the indicators, the Power menu, etc.

[kkeijzer]

I think in this case, the «app» to take screenshots in x seconds will be handful. So you can take screenshots of the indicators, the Power menu, etc.

But can we so easily run a command like itemGrabber.capture(shell); from something that is not the shell?

The PinePhone stack doesn't use the mirclient API, so you can't use something like mirscreencast to take the screenshot. There is also no adb running because there's no libhybris, so you can't use that for the screenshot either. So far the only way anyone has found to take screenshots on Wayland is directly from the shell. Even catting /dev/fb0 doesn't work.

[mariogrip]

I think this is fine tbh, android devices seem to do and imo i think it looks fine.

[mariogrip]

I think in this case, the «app» to take screenshots in x seconds will be handful. So you can take screenshots of the indicators, the Power menu, etc.

But can we so easily run a command like itemGrabber.capture(shell); from something that is not the shell?

A way would be to have a dbus command into Lomiri to grab screenshot

[kkeijzer]

By the way, is it expected for the CI to fail on this?

[dobey]

By the way, is it expected for the CI to fail on this?

There are tests for the shutdown dialog, and you've changed it, breaking the tests. So you'll need to fix the tests as well.

[dobey]

Also, let's please not turn this simple fix into a massively complicated discussion about re-design of shell components, adding more features for screenshots, and what even is a screenshot anyway. This solves an immediate problem in a simple way, and is consistent with the most used mobile phone OS. We can solve bigger problems later.

[kkeijzer]

There are tests for the shutdown dialog, and you've changed it, breaking the tests. So you'll need to fix the tests as well.

Ok, so this is in tests/qmltests/Components/tst_Dialogs.qml I suppose?

[dobey]

There are tests for the shutdown dialog, and you've changed it, breaking the tests. So you'll need to fix the tests as well.

Ok, so this is in tests/qmltests/Components/tst_Dialogs.qml I suppose?

No, though that probably needs some tests added for the button, given the context of the change you made there.

The failing tests you can find by searching for FAIL! on https://ci.ubports.com/blue/rest/organizations/jenkins/pipelines/ubports/pipelines/unity8/branches/PR-329/runs/2/nodes/26/steps/37/log/?start=0

They are in the OrientedShell test.

[kkeijzer]

Well, I'm getting closer. Now only the arm64 build fails.

[kkeijzer]

So, the difference I'm seeing in the logs is this:

tst_DragHandle.qml:38: ReferenceError: MouseTouchAdaptor is not defined

This happens only on the arm64 build but not on amd64 and armhf.

I haven't changed this file at all though.

[mariogrip]

So, the difference I'm seeing in the logs is this:

tst_DragHandle.qml:38: ReferenceError: MouseTouchAdaptor is not defined

This happens only on the arm64 build but not on amd64 and armhf.

I haven't changed this file at all though.

I think this is a issue with already flaky tests, I think those can be ignored in this case and should be fixed separably to this PR. Pinging @UniversalSuperBox for some input on this

[mariogrip]

Also, let's please not turn this simple fix into a massively complicated discussion about re-design of shell components, adding more features for screenshots, and what even is a screenshot anyway. This solves an immediate problem in a simple way, and is consistent with the most used mobile phone OS. We can solve bigger problems later.

I agree with this, we can always fix this later, its better to have it working but not like we want it then not working at all. This is a blocker on pinephones so we should at least make a simple fix and fix it later.

[kkeijzer]

Has there been any consensus found on this matter? I am personally indifferent to where we're going to put a button, but I would like a solution to be found at some point.

If we're not going to go forward with a button in the power menu, I could really use some help with adding one to an indicator or adding a second physical button combination (like Power + Volume Down) for screenshots so the PinePhone / PineTab can work as well.

[cibersheep]

Let me try something today and I get back to you

[cibersheep]

ubports/indicator-session#9
I don't know if this is correct. Is a start to add an entry in the indicator, but I don't know how to trigger a Dialog similar to the Shutdown dialog.

The idea could be opening a separate dialog for the user to chose between taking a screen shot in the moment or in x seconds

Whta do you think?

[kkeijzer]

Initially I'd say that we don't need any pop-up at all. It should just take a screenshot similar to what VolUp+VolDown does on Halium-ported phones. Nothing fancy.

Advanced screenshot functionality is probably best in a separate application that can be written in a (much) later stage. For now we should just focus on a clean way to get a simple screenshot without hardware buttons.

The next step would - in my opinion - be adding a second key combination for hardware screenshots so the PinePhone / PineTab can be supported that way as well. I briefly looked into that a while back, but I'm just not very familiar with QML.

And then finally we could make an application that allows taking screenshots of certain regions, with timeouts, and so on.

[dobey]

Has there been any consensus found on this matter? I am personally indifferent to where we're going to put a button, but I would like a solution to be found at some point.

Regardless of any future solutions, currently merges are frozen until OTA-13 is released this week. PinePhone won't be the only device affected either.

[dobey]

Thanks for this change. We can merge it now, so Pinephone/Pinetab users can start taking screenshots without hacking up the system, and we can re-examine design around the dialog and how else to present a screenshot action on screen in the future.

[Bolly]

Checked this on a BQM10FHD running RC 2020 W-40.
It works.
Thanks @kkeijzer !!!

[gbdomubpkm]

when will the mechanism be deployed on the Pinephone? I don't see it (in power menu) in last dev : take screenshots will be very useful for bug reports i think.

[dobey]

when will the mechanism be deployed on the Pinephone? I don't see it (in power menu) in last dev : take screenshots will be very useful for bug reports i think.

Unfortunately there seems to be something causing some major issues with the unity8 build and tests on the edge/wayland builds, so as soon as that is resolved this will appear on pine images the following day.

[TellingBerto]

Device: VollaPhone
Channel: rc
Build: 2020-W43

Results:

1. Button for Screenshot has been added to the power off screen.
2. Screenshot is taken and saved on the device when pressing the button Screenshot.

[cibersheep]

• Pull down the indicator session > Shutdown
• On the Dialog tap on Screenshot

The screenshot is always of the indicator

[dobey]

@cibersheep I don't think I would consider this a regression, and it's also not clear what one expects to be taking a screenshot of at that point if not the indicator. But if you think it's an issue and the indicator should be hidden before the shutdown dialog is opened, could you open another ticket for that. I don't think it should block the OTA. :)

[kugiigi]

I agree with Rodney. The implementation is in the power menu and not directly in the indicator. The proper use is using the power button. If the indicator has to be closed then the screenshot button should be in the indicators.

[dobey]

If the indicator has to be closed then the screenshot button should be in the indicators.

No, it's more about whether the indicator should be closed when the menu item is tapped, in this case. Just like when you tap any of the other entries in the session indicator.

[cibersheep]

• I don't think this should stop the OTA as is just cosmetic.
• I think we should separate the soft button menu with the shut down menu as the screeshot button doesn't belong to the shutdown menu (issue opened)