Screenshot popup alert #675

iamutkarshtiwari · 2016-04-06T14:56:29Z

This feature add a screenshot alert to Sugar.
On pressing Alt+1 key combination or selecting 'Take a screenshot' option from bottom toolbar
a popup alert appears where user could can the default name of that screenshot.
The popup can be dismissed by clicking 'X' button or by pressing 'Escape' key.
Enter can also be used to save and dismiss the alert.

iamutkarshtiwari · 2016-04-06T15:03:16Z

@quozl @davelab6 Please try this patch and share your suggestions. @samdroid-apps You might like it ;)

quozl · 2016-04-06T21:45:31Z

src/jarabe/screenshotpanel/gui.py

+    preview_surface.write_to_png(preview_str)
+
+    return dbus.ByteArray(preview_str.getvalue())
+


Extra blank lines, please remove.

quozl · 2016-04-06T21:50:33Z

Looks great.

Worried about the commit message; during review it wasn't clear what you had done and why.

iamutkarshtiwari · 2016-04-08T06:08:11Z

Fixed the commit message. Fixed the blank lines. Updated the copyright.

On pressing <Alt>+1 key or selecting 'Take a screenshot' option from bottom toolbar generates a popup alert where user can change the default name of that screenshot. The popup can be dismissed by clicking 'X' button or by pressing 'Escape' key. Currently there is no notification on screen capture in Sugar. The user wants to rename the screenshot, he has to go back to Journal to do it.

quozl · 2016-04-12T00:00:15Z

I'm hopeful of receiving feedback from others on whether the dialog should be styled to look like either;

the wireless password prompt (no X or tick decorators), or
the control panel, view source, or help modal windows.

quozl · 2016-04-13T01:23:49Z

It is proving difficult to get any feedback, sorry for the delay.

quozl · 2016-04-13T01:57:19Z

quozl · 2016-04-13T02:26:52Z

Unwelcome side-effect; when a menu palette is open, such as the central icon on the home view, the alt+1 key causes the palette to close.

davelab6 · 2016-04-13T02:32:49Z

Unwelcome side-effect; when a menu palette is open, such as the central icon on the home view, the alt+1 key causes the palette to close.

Is that a GTK level issue?

quozl · 2016-04-13T02:34:44Z

Is that a GTK level issue?

No, it's a Sugar Python code issue. GTK will do whatever we tell it to do.

davelab6 · 2016-04-13T03:58:32Z

No, it's a Sugar Python code issue. GTK will do whatever we tell it to do.

Should it be solved in this PR?

quozl · 2016-04-13T04:19:16Z

Should it be solved in this PR?

Yes, because this PR causes the regression. It doesn't happen without this PR.

samdroid-apps · 2016-05-12T10:58:36Z

Maybe we could move the screenshotpopup module into jarabe.view. I don't want to clutter the root jarabe namespace.

Also, please add the screenshpotpopup directory to the parent's Makefile.am's SUBDIRS.

samdroid-apps · 2016-05-12T11:01:40Z

Also also add the new directory to configure.ac

iamutkarshtiwari · 2016-05-12T11:02:02Z

@samdroid-apps I'll do as directed but I need your little help with this patch. Whenever I press the +<1> key combination more than once, more than one popup gets generated over one another.

Whereas what I expect is that it should generate only one popup even if the user presses the key combination more than once.

samdroid-apps · 2016-05-12T11:07:24Z

Yes, only one of these popups should be shown at a time. You might be interested in sharing this logic between the screenshot and settings popups.

Visually, I think it has regressed. The toolbar buttons were more consistent than using the traditional buttons as you do now.

Additionally, you did not fix any issues that @quozl posted in: #675 (comment)

You may be interested in @AbrahmAB's patch to move most of the modal popup logic into a common base class. You should co-ordinate. I don't want any new modal popups that do more of this copy paste code reuse.

Module is now inherited from sugar3.graphics.popwindow Thumbnail size fixed according to the aspect ratio.

iamutkarshtiwari · 2016-06-14T13:33:56Z

@samdroid-apps Module rewritten using Abhijit's popup module.

samdroid-apps · 2016-06-17T12:07:59Z

src/jarabe/screenshotpanel/gui.py

@@ -0,0 +1,342 @@
+#!/usr/bin/env python


Can you move this to jarabe.gui.screenshotpanel or something? Having a whole folder for 1 file is unnecessary.

I'll do it but before that we need to patch that grey rectangle bug.

It is blocking me from testing :)

Panel file now moved to jarabe/

iamutkarshtiwari · 2016-06-17T14:44:14Z

@samdroid-apps Issue resolved ;) 👍

devAbnull · 2016-06-17T19:34:48Z

Unwelcome effect: After taking Screenshot of Control Panel and while typing in the text entry of the pop-up, the search_entry of Control Panel toolbar steals the focus!
The user needs to close the Control Panel to write in the text-entry of the pop-up.

samdroid-apps · 2016-06-17T21:54:45Z

Looks so good. Here is my feedback:

Sorry for my miscommunication. I intended to say jarabe.view.screenshotpopup, not jarabe.gui.
Instead of having the yes/no buttons, can you just add a tick ("Save Screenshot") button to the toolbar?
Then entry is very short. Can you put it above the screenshot, with the label to the left of the entry. Like:


    | Save Screenshot         [Save] [X] |
    |-----------------------------------|
    | Title:  |Screenshot Of XYZ......|  |
    | .________________________________. |
    | |THUMBNAIL                       | |
    | |                                | |
    | |                                | |
    | |________________________________| |

The thumbnail seems static for me. I took a screenshot of the home view, and got the right thumbnail. Then I opened Log, took a screenshot and I got the same homeview thumbnail. It saved the correct screenshot though.
I can reproduce Abhijit's issue with the settings popup
The thumbnail looks squashed on my laptop. My screen is a 16:9 aspect ratio, not 4:3. It would be interesting to fix it.

samdroid-apps · 2016-06-17T21:55:42Z

src/jarabe/gui.py

+            del jobject
+
+
+def generate_thumbnail(screenshot_surface):


This is coppied from somewhere right (which is fine)?

If so, can you delete it in the other place? I don't want to have 2 identical generate_thumbnail functions lying around the codebase.

Yes. This function is a modified version of the original generate_thumbnail() so the code is unique in itself.

iamutkarshtiwari · 2016-08-20T13:46:08Z

@samdroid-apps Is there any way to fix that deadlock of this popup with Settings windows? I am eager on improving this PR. I'll rework the UI as you suggested.

quozl · 2016-11-09T03:41:53Z

@iamutkarshtiwari, any news on this one?

iamutkarshtiwari · 2016-11-09T07:37:12Z

@quozl There were some UI changes proposed by Sam which I haven't looked into yet (been busy with other projects) but will soon get back to it.

i5o · 2016-11-15T02:19:45Z

@iamutkarshtiwari if you need help let me know.

We can also make a GCI Task for this feature.

iamutkarshtiwari · 2016-11-15T14:06:17Z

@i5o I would really love to get this PR fixed. Sure, we can make a GCI task for this feature.

Things to be done-

Changes to be made as suggested by Screenshot popup alert #675 (comment)
This popup seems to get deadlocked by 'settings' popup - Screenshot popup alert #675 (comment). We can get this fixed as well.

Let me know if you need more info.

quozl · 2016-11-15T21:05:47Z

Handing an incomplete pull request to a student for GCI seems unfair to me. Huge complexity to deal with. I'm fine with you finishing up your work and merging it. 😄

iamutkarshtiwari · 2017-04-25T20:35:47Z

@quozl @tony37 Give me some time, I'll patch this one soon.

quozl · 2017-04-25T21:50:30Z

Thanks. Also please list what was changed from feedback.

jamessandy · 2017-11-15T20:20:32Z

@iamutkarshtiwari they is a way of fixing the deadlock

iamutkarshtiwari · 2017-11-15T20:57:01Z

@jamessandy Thanks for your interest in this PR. Your suggestions are welcome :)

jamessandy · 2017-11-15T21:06:55Z

Lets begin

…or `PrtSc`

iamutkarshtiwari force-pushed the feature-ScreenshotPopup branch from eab7d61 to 52e1953 Compare April 6, 2016 20:15

quozl reviewed Apr 6, 2016
View reviewed changes

iamutkarshtiwari force-pushed the feature-ScreenshotPopup branch from 52e1953 to dc8f759 Compare April 7, 2016 05:30

iamutkarshtiwari force-pushed the feature-ScreenshotPopup branch from dc8f759 to 02636de Compare April 8, 2016 22:44

samdroid-apps added feature design needs work labels May 8, 2016

toolbar and buttons updated

a04d6ad

Screenshot module modified

139ea38

Module is now inherited from sugar3.graphics.popwindow Thumbnail size fixed according to the aspect ratio.

samdroid-apps reviewed Jun 17, 2016
View reviewed changes

Screenshot panel

39062bf

Panel file now moved to jarabe/

samdroid-apps reviewed Jun 17, 2016
View reviewed changes

quozl force-pushed the master branch from 7473e7a to 5a1e02a Compare February 25, 2019 19:29

chimosky referenced this pull request in chimosky/sugar May 25, 2019

[WIP] Adding of screenshot panel for screenshots on pressing Alt 1 …

0d90290

…or `PrtSc`

		preview_surface.write_to_png(preview_str)

		return dbus.ByteArray(preview_str.getvalue())

Screenshot popup alert #675

Are you sure you want to change the base?

Screenshot popup alert #675

Conversation

iamutkarshtiwari commented Apr 6, 2016 • edited

iamutkarshtiwari commented Apr 6, 2016

quozl Apr 6, 2016

Choose a reason for hiding this comment

quozl commented Apr 6, 2016

iamutkarshtiwari commented Apr 8, 2016

quozl commented Apr 12, 2016

quozl commented Apr 13, 2016

quozl commented Apr 13, 2016 • edited

quozl commented Apr 13, 2016 • edited by iamutkarshtiwari

davelab6 commented Apr 13, 2016

quozl commented Apr 13, 2016

davelab6 commented Apr 13, 2016

quozl commented Apr 13, 2016

samdroid-apps commented May 12, 2016 • edited by iamutkarshtiwari

samdroid-apps commented May 12, 2016

iamutkarshtiwari commented May 12, 2016 • edited

samdroid-apps commented May 12, 2016

iamutkarshtiwari commented Jun 14, 2016

samdroid-apps Jun 17, 2016

Choose a reason for hiding this comment

iamutkarshtiwari Jun 17, 2016

Choose a reason for hiding this comment

samdroid-apps Jun 17, 2016

Choose a reason for hiding this comment

iamutkarshtiwari commented Jun 17, 2016 • edited

devAbnull commented Jun 17, 2016 • edited

samdroid-apps commented Jun 17, 2016

samdroid-apps Jun 17, 2016

Choose a reason for hiding this comment

iamutkarshtiwari Jul 19, 2016

Choose a reason for hiding this comment

iamutkarshtiwari commented Aug 20, 2016 • edited

quozl commented Nov 9, 2016

iamutkarshtiwari commented Nov 9, 2016

i5o commented Nov 15, 2016

iamutkarshtiwari commented Nov 15, 2016

quozl commented Nov 15, 2016

iamutkarshtiwari commented Apr 25, 2017

quozl commented Apr 25, 2017

jamessandy commented Nov 15, 2017

iamutkarshtiwari commented Nov 15, 2017

jamessandy commented Nov 15, 2017

iamutkarshtiwari commented Apr 6, 2016 •

edited

quozl commented Apr 13, 2016 •

edited

quozl commented Apr 13, 2016 •

edited by iamutkarshtiwari

samdroid-apps commented May 12, 2016 •

edited by iamutkarshtiwari

iamutkarshtiwari commented May 12, 2016 •

edited

iamutkarshtiwari commented Jun 17, 2016 •

edited

devAbnull commented Jun 17, 2016 •

edited

iamutkarshtiwari commented Aug 20, 2016 •

edited