Skip to content

Commit

Permalink
[WIP] Adding of screenshot panel for screenshots on pressing Alt 1
Browse files Browse the repository at this point in the history
…or `PrtSc`
  • Loading branch information
chimosky committed May 25, 2019
1 parent a83257b commit 0d90290
Show file tree
Hide file tree
Showing 5 changed files with 363 additions and 5 deletions.
6 changes: 3 additions & 3 deletions extensions/deviceicon/display.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

from jarabe.frame.frameinvoker import FrameWidgetInvoker
from jarabe.model import brightness
from jarabe.model.screenshot import take_screenshot
from jarabe.view.screenshotpopup import ScreenshotPanel
from jarabe import frame


Expand Down Expand Up @@ -90,7 +90,7 @@ def __init__(self, text, icon_name):
icon = Icon(pixel_size=style.SMALL_ICON_SIZE)
icon.props.icon_name = icon_name
icon.props.xo_color = XoColor('%s,%s' % (style.COLOR_WHITE.get_svg(),
style.COLOR_BUTTON_GREY.get_svg()))
style.COLOR_BUTTON_GREY.get_svg()))
icon.show()

label = Gtk.Label(text)
Expand Down Expand Up @@ -228,7 +228,7 @@ def __screenshot_cb(self, palette):
def __take_screenshot_cb(self, frame_):
if frame_.is_visible():
return True
take_screenshot()
panel = ScreenshotPanel()
frame_.show()
return False

Expand Down
4 changes: 2 additions & 2 deletions extensions/globalkey/screenshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from jarabe.model.screenshot import take_screenshot
from jarabe.view.screenshotpopup import ScreenshotPanel

BOUND_KEYS = ['<alt>1', 'Print']


def handle_key_press(key):
take_screenshot()
panel = ScreenshotPanel()
26 changes: 26 additions & 0 deletions src/jarabe/desktop/activitychooser.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@


class TitleBox(Gtk.Toolbar):
'''
Title box at the top of the pop-up window.
Title and close button are added to the box and as needed
more widgets can be added using self.add_widget method.
This box is optional as the inherited class can remove this
block by setting the self._set_title_box to False.
'''

def __init__(self):
Gtk.Toolbar.__init__(self)

Expand All @@ -52,9 +60,16 @@ def __init__(self):
tool_item.show()

def set_title(self, title):
'''
setter function for 'title' property.
Args:
title (str): title for pop-up window
'''
self._label.set_markup('<b>%s</b>' % title)
self._label.show()

title = GObject.Property(type=str, setter=set_title)


_AUTOSEARCH_TIMEOUT = 1000

Expand Down Expand Up @@ -149,6 +164,17 @@ def __init__(self):

self.show()

def get_title_box(self):
'''
Getter method for title-box
Returns:
self._title_box (): Title or Tool Box
'''
return self._title_box

title_box = GObject.Property(type=str, getter=get_title_box)

def __close_button_clicked_cb(self, button):
self.destroy()

Expand Down
1 change: 1 addition & 0 deletions src/jarabe/view/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ sugar_PYTHON = \
launcher.py \
palettes.py \
pulsingicon.py \
screenshotpopup.py \
service.py \
tabbinghandler.py \
viewsource.py \
Expand Down
Loading

6 comments on commit 0d90290

@chimosky
Copy link
Owner Author

@chimosky chimosky commented on 0d90290 May 25, 2019

Choose a reason for hiding this comment

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

@quozl I'm working on completing sugarlabs#675.

So far, pressing the alt 1 key brings up the screen shot panel but there's a few seconds relapse where a black child window appears on the screen before the screen shot panel comes up and this black child window also shows in the screenshot, yet to ascertain why this is happening.

As per your comments some of the issues have already been fixed, some still there.

While we wait for feedback ... tested on Ubuntu 16.04 with Sugar 0.108.

appearance issues

  • the "Name" label is unnecessary; we don't have a name label in the Journal detail view, or the activity toolkit text entry for journal object; it is obvious from context that it is a journal entry name,

  • if after consensus the "Name" label is kept,

  • it is yellow, it should be white,

  • it is too close to the text entry,

  • it might be to the left of the text entry, like "Server:' in My Settings, Network, Collaboration,

  • the preview image is distorted; because the aspect ratio of the preview doesn't match the aspect ratio of the display; you've coded 200:160 (1.5) instead of detecting the display, (you've also used these constants at least twice in the code without using a variable for them),

interaction issues - keyboard

  • the dialog does not respond to the Esc key, it must dismiss the dialog and cancel the screenshot, see Hotkeys,

  • the dialog does not respond to the Enter key, it must accept the dialog and save the screenshot,

interaction issues - other windows

  • while the dialog is visible, the frame can be shown with the f6 key, but does not react to input,

  • while the dialog is visible, the f1, f2, f3 keys will hide the dialog and switch view, but the view does not react to input, the user is trapped with no obvious way to fix, but any key press shows the dialog again,

  • while in the home view, activating the My Settings control panel (alt-shift-m), and then pressing alt+1 locks the display; the control panel cannot be closed, workaround is to restart display manager service lightdm restart,

  • while in the home view, activating the Help window (alt+shift+h), and then pressing alt+1 locks the display, as above.

Hope that helps!

One regression I noticed though is that once the alt 1 key is pressed and the screenshot panel comes up, pressing the F1-3 keys don't work.

@quozl
Copy link

@quozl quozl commented on 0d90290 May 27, 2019

Choose a reason for hiding this comment

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

Wild guess; the instance of ScreenshotPanel is not kept; it is automatically deleted when the function that created it has returned.

Remind me, where's the design for this feature, and has it been updated recently? I don't like not involving everyone else in this kind of change.

@chimosky
Copy link
Owner Author

Choose a reason for hiding this comment

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

Remind me, where's the design for this feature, and has it been updated recently?

Original PR here

I don't like not involving everyone else in this kind of change.

I plan on involving everyone, I wanted to get your reply on your comments as I'd planned to implement them before sending a PR.

@quozl
Copy link

@quozl quozl commented on 0d90290 May 29, 2019

Choose a reason for hiding this comment

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

http://wiki.sugarlabs.org/go/Features/Sugar_Screenshot_Dialog_Popup is the feature page. It doesn't look as recent as the PR you refer to. I think I've replied. Do you have any specific questions?

@chimosky
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'd like you to look at the items you listed in the PR - I listed them above too -, and confirm which ones have been ticked off the list.

@quozl
Copy link

@quozl quozl commented on 0d90290 Jun 3, 2019

Choose a reason for hiding this comment

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

Could you fix the black child window problem first?

Please sign in to comment.