! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Improvements to the uploaded Imgur image dialog
Status:
RESOLVED: FIXED
Product:
Xfce4-screenshooter
Component:
General

Comments

Description Arthur Jansen 2018-12-13 15:21:59 CET
Created attachment 8178 
New imgur dialog

On this page: https://wiki.xfce.org/contribute/easybugs, the task 'Code a new design for the Screenshooter imgur windows' was listed and a mockup was provided (http://i.imgur.com/DutqdYo.png). Since the current dialog does not look ver good imo, I thought I could try doing it.
I did not add a 'Share' button because I don't know how I could implement that.
I am quite new to programming with GTK so if I did anything wrong or could improve anything, please let me know.
Comment 1 Andre Miranda editbugs 2018-12-15 03:00:45 CET
Created attachment 8182 
header screenshot

Hi Arthur,
Your implementation looks awesome, I would merge it right away, only if you had sent a patch with commit info (git format-patch).

Before you do that, if I'm not asking too much, there is a couple of things to be improved:
- The dialog should not start too wide.
- I would decrease the space between buttons, 8 seems enough (perhaps a grid for the first tab would help).
- In "Embed into code" tab, I would align the labels to the left (set labels horizontal alignment to 0) and rename "Show" as "Size", makes more sense and is consistent to "Image" tab.
- The href attribute of the "Link to full size" snippet seems to ignore the image size, is that intentional?
- This one is a bit tricky: the dialog's heading looks a bit different than the others, because it does not use XfceHeading. We could use it from the glade xml, I just never tried. The other approach is to use css styles.

Let me know if you interested in those improvements, if not, no problem, I can implement them after merging your patch.

Thanks!
Comment 2 Arthur Jansen 2018-12-15 13:51:31 CET
Hello

Thanks for the comments, I will try to make these improvements.

For the "Link to full size" function: yes, this is intentional, I interpreted this as showing the image at the selected size and making it a link to the full-size image. This is also what I would personally expect from "Link to full size" but of course I can change it if you have a different idea.
Comment 3 Arthur Jansen 2018-12-15 15:30:50 CET
Created attachment 8183 
Updated patch
Comment 4 Andre Miranda editbugs 2018-12-16 05:39:09 CET
Wow, that was fast, all looks good now, thanks.
With regards to "Link to full size", I have no strong opinion, let's keep it as is.

I made some adjustments to your patch, let me know if I broke something before we push this to master:
https://git.xfce.org/users/andre/xfce4-screenshooter/log/?h=bug-14973
Comment 5 Arthur Jansen 2018-12-16 22:02:01 CET
I just tested it and it is working fine for me.
However, I am getting some run-time warnings when I close the dialog. I don't know if this is a problem or can be ignored.
Comment 6 Andre Miranda editbugs 2018-12-16 22:29:37 CET
I'm not getting any warnings, please copy and paste them here.
Comment 7 Arthur Jansen 2018-12-16 23:09:13 CET
Here are the warnings, they do not appear when I comment line 82 from screenshooter-imgur-dialog.c (g_object_unref (self->window);).

(xfce4-screenshooter:21551): GLib-GObject-CRITICAL **: 23:02:26.696: g_object_set_qdata: assertion 'G_IS_OBJECT (object)' failed
(xfce4-screenshooter:21551): GLib-GObject-WARNING **: 23:02:26.696: instance with invalid (NULL) class pointer
(xfce4-screenshooter:21551): GLib-GObject-CRITICAL **: 23:02:26.696: g_signal_handlers_destroy: assertion 'G_TYPE_CHECK_INSTANCE (instance)' failed
(xfce4-screenshooter:21551): GLib-GObject-WARNING **: 23:02:26.696: instance with invalid (NULL) class pointer
(xfce4-screenshooter:21551): GLib-GObject-CRITICAL **: 23:02:26.696: g_signal_handlers_destroy: assertion 'G_TYPE_CHECK_INSTANCE (instance)' failed
Comment 8 Andre Miranda editbugs 2018-12-16 23:39:40 CET
Please replace g_object_unref with gtk_widget_destroy, let me know if the warnings are gone.

If not, please build with debug symbols (./autogen.sh --enable-debug=yes) and run as "G_DEBUG=fatal_warnings gdb src/xfce4-screenshooter". Once gdb pauses, get the backtrace with "bt" and post here.
Comment 9 Arthur Jansen 2018-12-16 23:59:40 CET
Created attachment 8185 
GDB bt output

Replacing it with gtk_widget_destroy fixed it.
I attached the gdb output in case you were still interested.
Comment 10 Git Bot editbugs 2018-12-17 00:04:41 CET
Arthur Jansen referenced this bugreport in commit 1a4ebe1ee23a12f5996187ec6060ab71963ffd11

Improved imgur dialog (Bug #14973)

https://git.xfce.org/apps/xfce4-screenshooter/commit?id=1a4ebe1ee23a12f5996187ec6060ab71963ffd11
Comment 11 Andre Miranda editbugs 2018-12-17 00:05:37 CET
Merged, many thanks!
Comment 12 Git Bot editbugs 2019-05-22 08:41:32 CEST
Landry Breuil referenced this bugreport in commit e14a75d80e3233ab850f27908031162f897f5f25

Add missing NULL as last parameter of xfce_titled_dialog_new_with_buttons() (Bug #14973)

https://git.xfce.org/apps/xfce4-screenshooter/commit?id=e14a75d80e3233ab850f27908031162f897f5f25

Bug #14973

Reported by:
Arthur Jansen
Reported on: 2018-12-13
Last modified on: 2019-05-22

People

Assignee:
Jérôme Guelfucci
CC List:
1 user

Version

Attachments

New imgur dialog (62.27 KB, patch)
2018-12-13 15:21 CET , Arthur Jansen
no flags
header screenshot (13.84 KB, image/png)
2018-12-15 03:00 CET , Andre Miranda
no flags
Updated patch (53.85 KB, patch)
2018-12-15 15:30 CET , Arthur Jansen
no flags
GDB bt output (2.03 KB, text/plain)
2018-12-16 23:59 CET , Arthur Jansen
no flags

Additional information