[Crash-catcher] [PATCH] gui rewrite

[Jiri Moskovcak]

Hi,
this is a patch to change the main window according to desing from 
Mairin Duffy. The screenshots can be found here:

http://jmoskovc.fedorapeople.org/icons_as_status.png

in case someone just want to see it.

Please review,
Jirka

[Karel Klic]

Hi,
I just run tested the patch, and it looks good. Please see the comments 
below. I also commented on some parts which are not part of the new GUI, 
but just the user's perspective; maybe I should resend it in a separate 
email.

1) Main menu -> Help -> View log does nothing.
If it's not implemented yet, I would hide it to avoid confusion.

2) Help button on the main window shows About dialog.
We have no manual, no help texts. I would hide the button for the 
release version to avoid confusion.

3) "Copy to Clipboard" button when used on kerneloops:
Traceback (most recent call last):
   File "/usr/share/abrt/CCMainWindow.py", line 322, in on_b_copy_clicked
     ("Comment:", dump.comment),
AttributeError: Dump instance has no attribute 'comment'

4) "Copy to Clipboard" is enabled on kerneloops, but it is disabled on 
other regular crash (seems it's disabled on unreported crashes). I see 
no difference in the "Details" panel which would justify the 
disablement. Please enable it for all crashes.

5) When kerneloops is selected, there are some problems in the Details 
panel:
  a) The heading is "Kerne Crash" (missing letter)
  b) Under the heading there is a line with word "kernel", but when 
other packages are selected, the line contains package name with version 
number and release number. So for the kerneloops it should probably also 
show the version and release numbers of the kernel.
  c) the panel shows "Command: not_applicable". When the command does 
not make sense for kerneloops, it should be hidden. Or, it can say "not 
applicable" without the underscore.
  d) The "Reason:" part shows this text:
"---------------[cut here]-------------------"
It is very confusing, as this is not a "reason". I do not know if the 
Reason field makes sense for kerneloopses, but it clearly does not make 
sense in some cases.

6) In Details panel uppercase/lowercase is not consistent: compare 
"Latest _C_rash" and "Crash _c_ount"

7) Main menu -> Help -> About dialog shows two lines:
(C) 2009 Red Hat, Inc.
(C) 2010 Red Hat, Inc.
The copyright lines should not be duplicated for every year.
I would merge them into one line:
(C) 2009, 2010 Red Hat, Inc.

8) Main menu -> Edit -> Plugins dialog:
  a) I would rename "Not loaded plugins" to "Unloaded plugins"
  b) A header "Name" on the top is misleading, as the list does not 
contain names. It should be removed.
  c) Plugin details are missing for unloaded plugins. From user's 
perspective there is no reason why those should be missing.
  d) Plugin details should not include "author" line. The line suggests 
user to send email to the provided email address and ask for help. The 
"Web Site" field is more relevant: if user wants to know more about some 
particular plugin, he visits the website and looks for help there.
  e) Kerneloops plugin shows website 
"https://people.redhat.com/aarapov". ABRT team provides support for the 
kerneloops plugin, so the link should probably direct user to ABRT wiki
  f) Version numbers displayed for various plugins are wrong. We do not 
update them even when the plugin is completely changed. So for our 
"core" plugins, I would synchronize the plugin version with ABRT version.
  g) Bugzilla plugin -> Configure contains "No SSL verify" checkbox.
  It should be "SSL verify" to avoid mandatory exercises in basic logic.
  h) Bugzilla plugin -> Configure contains "Login:" field. Several users 
were already confused by it and wrote wrong value into it, we should 
change it to "Login email:"

9) Main menu -> Edit -> Preferences is still visible even when it's not 
implemented. It already confused many users (they reported it as a bug). 
We should hide it at least for official RPMs.

10) When abrt-gui is started, the focus in on the "Application" column 
heading. The focus should be on the crash list, so user can instantly 
use arrow keys to select a crash.

11) When you press Tab key to move focus from one control to another, it 
jumps to some hidden control, but not to the Details panel. When the 
focus is on the crash list and user pressed Tab, it should jump to the 
Details panel (which is scrollable when the window is too small), and 
form the Details panel it should jum to the Delete button.

12) The "Help" and "Close" buttons on the main window should be moved

upwards to retain the padding between the controls and the edge of the 
window. The buttons are too close to the status bar.
Also the "Report" button is placed too close to the right border of the 
window. It should be aligned with other controls.

Best regards,
Karel

On 03/25/2010 04:08 PM, Jiri Moskovcak wrote:
>  Hi,
>  this is a patch to change the main window according to desing from
>  Mairin Duffy. The screenshots can be found here:
>
>  http://jmoskovc.fedorapeople.org/icons_as_status.png
>
>  in case someone just want to see it.
>
>  Please review,
>  Jirka
>
>
>
>  _______________________________________________
>  Crash-catcher mailing list
>  Crash-catcher(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/crash-catcher

[Jiri Moskovcak]

On 03/30/2010 12:08 PM, Karel Klic wrote:
>  Hi,
>  I just run tested the patch, and it looks good. Please see the comments
>  below. I also commented on some parts which are not part of the new GUI,
>  but just the user's perspective; maybe I should resend it in a separate
>  email.
>
>  1) Main menu ->  Help ->  View log does nothing.
>  If it's not implemented yet, I would hide it to avoid confusion.
>  
ok, will hide it

>  2) Help button on the main window shows About dialog.
>  We have no manual, no help texts. I would hide the button for the
>  release version to avoid confusion.
>  
I think the about dialog is ok (for now), it contains ABRT's url where 
users can find some info.

>  3) "Copy to Clipboard" button when used on kerneloops:
>  Traceback (most recent call last):
>      File "/usr/share/abrt/CCMainWindow.py", line 322, in on_b_copy_clicked
>        ("Comment:", dump.comment),
>  AttributeError: Dump instance has no attribute 'comment'
>  
fixed

>  4) "Copy to Clipboard" is enabled on kerneloops, but it is
> disabled on
>  other regular crash (seems it's disabled on unreported crashes). I see
>  no difference in the "Details" panel which would justify the
>  disablement. Please enable it for all crashes.
>  
ok

>  5) When kerneloops is selected, there are some problems in the
> Details
>  panel:
>     a) The heading is "Kerne Crash" (missing letter) 
fixed
>     b) Under the heading there is a line with word "kernel",
> but when
>  other packages are selected, the line contains package name with version
>  number and release number. So for the kerneloops it should probably also
>  show the version and release numbers of the kernel. 
-  again, this problem is in kerneloops plugin

>     c) the panel shows "Command: not_applicable". When the
> command does
>  not make sense for kerneloops, it should be hidden. Or, it can say "not
>  applicable" without the underscore. 
- yes, it can, but will leave this for later

>     d) The "Reason:" part shows this text:
>  "---------------[cut here]-------------------"
>  It is very confusing, as this is not a "reason". I do not know if the
>  Reason field makes sense for kerneloopses, but it clearly does not make
>  sense in some cases.
>  
- kerneloops plugin

it's not a gui problem, this is more kerneloops plugin problem

>  6) In Details panel uppercase/lowercase is not consistent: compare
>  "Latest _C_rash" and "Crash _c_ount"
>  
- fixed

>  7) Main menu ->  Help ->  About dialog shows two lines:
>  (C) 2009 Red Hat, Inc.
>  (C) 2010 Red Hat, Inc.
>  The copyright lines should not be duplicated for every year.
>  I would merge them into one line:
>  (C) 2009, 2010 Red Hat, Inc.
>  
- fixed


>  8) Main menu ->  Edit ->  Plugins dialog:
>     a) I would rename "Not loaded plugins" to "Unloaded plugins"
>     b) A header "Name" on the top is misleading, as the list does not
>  contain names. It should be removed.
>     c) Plugin details are missing for unloaded plugins. From user's
>  perspective there is no reason why those should be missing.
>     d) Plugin details should not include "author" line. The line suggests
>  user to send email to the provided email address and ask for help. The
>  "Web Site" field is more relevant: if user wants to know more about some
>  particular plugin, he visits the website and looks for help there.
>     e) Kerneloops plugin shows website
>  "https://people.redhat.com/aarapov". ABRT team provides support for the
>  kerneloops plugin, so the link should probably direct user to ABRT wiki
>     f) Version numbers displayed for various plugins are wrong. We do not
>  update them even when the plugin is completely changed. So for our
>  "core" plugins, I would synchronize the plugin version with ABRT version.
>     g) Bugzilla plugin ->  Configure contains "No SSL verify" checkbox.
>     It should be "SSL verify" to avoid mandatory exercises in basic logic.
>     h) Bugzilla plugin ->  Configure contains "Login:" field. Several users
>  were already confused by it and wrote wrong value into it, we should
>  change it to "Login email:"
>  
- the plugin dialog is another issue, won't fix in this iteration

>  9) Main menu ->  Edit ->  Preferences is still visible even
> when it's not
>  implemented. It already confused many users (they reported it as a bug).
>  We should hide it at least for official RPMs.
>  
- ok

>  10) When abrt-gui is started, the focus in on the
> "Application" column
>  heading. The focus should be on the crash list, so user can instantly
>  use arrow keys to select a crash.
>  
- fixed

>  11) When you press Tab key to move focus from one control to another,
> it
>  jumps to some hidden control, but not to the Details panel. When the
>  focus is on the crash list and user pressed Tab, it should jump to the
>  Details panel (which is scrollable when the window is too small), and
>  form the Details panel it should jum to the Delete button.
>  
- fixed (mostly) - now the tab skips the detail's window and jumps 
directly to the buttons

>  12) The "Help" and "Close" buttons on the main
> window should be moved
>  upwards to retain the padding between the controls and the edge of the
>  window. The buttons are too close to the status bar.
>  Also the "Report" button is placed too close to the right border of the
>  window. It should be aligned with other controls.
>  
- fixed

The two attached patches should fix all of the issues. Please review.

>  Best regards,
>  Karel
>
>  On 03/25/2010 04:08 PM, Jiri Moskovcak wrote:
>    >  Hi,
>    >  this is a patch to change the main window according to desing from
>    >  Mairin Duffy. The screenshots can be found here:
>    >
>    >  http://jmoskovc.fedorapeople.org/icons_as_status.png
>    >
>    >  in case someone just want to see it.
>    >
>    >  Please review,
>    >  Jirka
>    >
>    >
>    >
>    >  _______________________________________________
>    >  Crash-catcher mailing list
>    >  Crash-catcher(a)lists.fedorahosted.org
>    >  https://fedorahosted.org/mailman/listinfo/crash-catcher
>  _______________________________________________
>  Crash-catcher mailing list
>  Crash-catcher(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/crash-catcher

[Karel Klic]

Thank you for the fixes. Please see below.

On 03/30/2010 04:12 PM, Jiri Moskovcak wrote:
>  On 03/30/2010 12:08 PM, Karel Klic wrote:
> > 2) Help button on the main window shows About dialog.
> > We have no manual, no help texts. I would hide the button for the
> > release version to avoid confusion.
>
>  I think the about dialog is ok (for now), it contains ABRT's url where
>  users can find some info. 
So wouldn't it be better to open the web page directly when user clicks 
the button? Several applications do this (pidgin, thunderbird, firefox).

> > 6) In Details panel uppercase/lowercase is not consistent:
> compare
> > "Latest _C_rash" and "Crash _c_ount"
>  - fixed 
The fix seems not to be a part of the attached patches.

> > 7) Main menu -> Help -> About dialog shows two lines:
> > (C) 2009 Red Hat, Inc.
> > (C) 2010 Red Hat, Inc.
> > The copyright lines should not be duplicated for every year.
> > I would merge them into one line:
> > (C) 2009, 2010 Red Hat, Inc.
>
>  - fixed 
The fix seems not to be a part of the attached patches.

> > 9) Main menu -> Edit -> Preferences is still visible even when it's not
> > implemented. It already confused many users (they reported it as a bug).
> > We should hide it at least for official RPMs.
> >
>  - ok 
The fix seems not to be a part of the attached patches.

When all crashes are deleted, the Details panel keeps displaying the 
details of the last crash (see the attached screenshot 
abrt-last-crash-deleted.png).

When the GUI is started with no crashes in the database, it shows the 
Details panel (see the attached screenshot abrt-no-crashes-avail.png). 
The panel should be probably hidden, or it can show something like "No 
crash has been detected by ABRT yet." instead of the empty controls. 
Also, the buttons "Delete", "Copy to Clipboard", and
"Report" should be 
disabled (Copy to Clipboard throws an exception, the others do nothing).

K.

[Jiri Moskovcak]

The new patch with all major problems fixed (hopefully).
Please review.

Jirka

On 03/30/2010 05:08 PM, Karel Klic wrote:
>  Thank you for the fixes. Please see below.
>
>  On 03/30/2010 04:12 PM, Jiri Moskovcak wrote:
> > On 03/30/2010 12:08 PM, Karel Klic wrote:
> >> 2) Help button on the main window shows About dialog.
> >> We have no manual, no help texts. I would hide the button for the
> >> release version to avoid confusion.
> >
> > I think the about dialog is ok (for now), it contains ABRT's url where
> > users can find some info.
>
>  So wouldn't it be better to open the web page directly when user clicks
>  the button? Several applications do this (pidgin, thunderbird, firefox).
>  
- maybe, but for now I'd prefer to leave it that way (or I can rename 
the button from "help" to "about" :))

> >> 6) In Details panel uppercase/lowercase is not consistent:
> compare
> >> "Latest _C_rash" and "Crash _c_ount"
> > - fixed
>  The fix seems not to be a part of the attached patches.
>  
- fixed now

> >> 7) Main menu -> Help -> About dialog shows two lines:
> >> (C) 2009 Red Hat, Inc.
> >> (C) 2010 Red Hat, Inc.
> >> The copyright lines should not be duplicated for every year.
> >> I would merge them into one line:
> >> (C) 2009, 2010 Red Hat, Inc.
> >
> > - fixed
>  The fix seems not to be a part of the attached patches.
>  
- fixed

> >
> >> 9) Main menu -> Edit -> Preferences is still visible even when it's
> not
> >> implemented. It already confused many users (they reported it as a bug).
> >> We should hide it at least for official RPMs.
> >>
> > - ok
>  The fix seems not to be a part of the attached patches.
>  
- I'll add the patch to do this to Fedora cvs and I keep it visible in git

>  When all crashes are deleted, the Details panel keeps displaying the
>  details of the last crash (see the attached screenshot
>  abrt-last-crash-deleted.png).
>  
- fixed

>  When the GUI is started with no crashes in the database, it shows
> the
>  Details panel (see the attached screenshot abrt-no-crashes-avail.png).
>  The panel should be probably hidden, or it can show something like "No
>  crash has been detected by ABRT yet." instead of the empty controls.
>  Also, the buttons "Delete", "Copy to Clipboard", and
> "Report" should be
>  disabled (Copy to Clipboard throws an exception, the others do nothing).
>  
- fixed

>  K.
>
>
>
>  _______________________________________________
>  Crash-catcher mailing list
>  Crash-catcher(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/crash-catcher

[Karel Klic]

Good work.
Reviewed by: Karel Klic <kklic(a)redhat.com&gt;

On 03/31/2010 10:56 AM, Jiri Moskovcak wrote:
>  The new patch with all major problems fixed (hopefully).
>  Please review.
>
>  Jirka
>
>  On 03/30/2010 05:08 PM, Karel Klic wrote:
> > Thank you for the fixes. Please see below.
> >
> > On 03/30/2010 04:12 PM, Jiri Moskovcak wrote:
> >> On 03/30/2010 12:08 PM, Karel Klic wrote:
> >>> 2) Help button on the main window shows About dialog.
> >>> We have no manual, no help texts. I would hide the button for the
> >>> release version to avoid confusion.
> >>
> >> I think the about dialog is ok (for now), it contains ABRT's url where
> >> users can find some info.
> >
> > So wouldn't it be better to open the web page directly when user clicks
> > the button? Several applications do this (pidgin, thunderbird, firefox).
> >
>
>  - maybe, but for now I'd prefer to leave it that way (or I can rename
>  the button from "help" to "about" :))
>
> >>> 6) In Details panel uppercase/lowercase is not consistent: compare
> >>> "Latest _C_rash" and "Crash _c_ount"
> >> - fixed
> > The fix seems not to be a part of the attached patches.
> >
>
>  - fixed now
>
> >>> 7) Main menu -> Help -> About dialog shows two lines:
> >>> (C) 2009 Red Hat, Inc.
> >>> (C) 2010 Red Hat, Inc.
> >>> The copyright lines should not be duplicated for every year.
> >>> I would merge them into one line:
> >>> (C) 2009, 2010 Red Hat, Inc.
> >>
> >> - fixed
> > The fix seems not to be a part of the attached patches.
> >
>
>  - fixed
>
> >>
> >>> 9) Main menu -> Edit -> Preferences is still visible even when it's
> not
> >>> implemented. It already confused many users (they reported it as a
> >>> bug).
> >>> We should hide it at least for official RPMs.
> >>>
> >> - ok
> > The fix seems not to be a part of the attached patches.
> >
>
>  - I'll add the patch to do this to Fedora cvs and I keep it visible in git
>
> > When all crashes are deleted, the Details panel keeps displaying the
> > details of the last crash (see the attached screenshot
> > abrt-last-crash-deleted.png).
> >
>
>  - fixed
>
> > When the GUI is started with no crashes in the database, it shows the
> > Details panel (see the attached screenshot abrt-no-crashes-avail.png).
> > The panel should be probably hidden, or it can show something like "No
> > crash has been detected by ABRT yet." instead of the empty controls.
> > Also, the buttons "Delete", "Copy to Clipboard", and
> "Report" should be
> > disabled (Copy to Clipboard throws an exception, the others do nothing).
> >
>
>  - fixed
>
> > K.

[Jiri Moskovcak]

On 03/31/2010 12:04 PM, Karel Klic wrote:
>  Good work.
>  Reviewed by: Karel Klic<kklic(a)redhat.com>
>  
Pushed to the git repo, thank you.

J.


>  On 03/31/2010 10:56 AM, Jiri Moskovcak wrote:
> > The new patch with all major problems fixed (hopefully).
> > Please review.
> >
> > Jirka
> >
> > On 03/30/2010 05:08 PM, Karel Klic wrote:
> >> Thank you for the fixes. Please see below.
> >>
> >> On 03/30/2010 04:12 PM, Jiri Moskovcak wrote:
> >>> On 03/30/2010 12:08 PM, Karel Klic wrote:
> >>>> 2) Help button on the main window shows About dialog.
> >>>> We have no manual, no help texts. I would hide the button for the
> >>>> release version to avoid confusion.
> >>>
> >>> I think the about dialog is ok (for now), it contains ABRT's url where
> >>> users can find some info.
> >>
> >> So wouldn't it be better to open the web page directly when user clicks
> >> the button? Several applications do this (pidgin, thunderbird, firefox).
> >>
> >
> > - maybe, but for now I'd prefer to leave it that way (or I can rename
> > the button from "help" to "about" :))
> >
> >>>> 6) In Details panel uppercase/lowercase is not consistent: compare
> >>>> "Latest _C_rash" and "Crash _c_ount"
> >>> - fixed
> >> The fix seems not to be a part of the attached patches.
> >>
> >
> > - fixed now
> >
> >>>> 7) Main menu ->  Help ->  About dialog shows two lines:
> >>>> (C) 2009 Red Hat, Inc.
> >>>> (C) 2010 Red Hat, Inc.
> >>>> The copyright lines should not be duplicated for every year.
> >>>> I would merge them into one line:
> >>>> (C) 2009, 2010 Red Hat, Inc.
> >>>
> >>> - fixed
> >> The fix seems not to be a part of the attached patches.
> >>
> >
> > - fixed
> >
> >>>
> >>>> 9) Main menu ->  Edit ->  Preferences is still visible even when
> it's not
> >>>> implemented. It already confused many users (they reported it as a
> >>>> bug).
> >>>> We should hide it at least for official RPMs.
> >>>>
> >>> - ok
> >> The fix seems not to be a part of the attached patches.
> >>
> >
> > - I'll add the patch to do this to Fedora cvs and I keep it visible in git
> >
> >> When all crashes are deleted, the Details panel keeps displaying the
> >> details of the last crash (see the attached screenshot
> >> abrt-last-crash-deleted.png).
> >>
> >
> > - fixed
> >
> >> When the GUI is started with no crashes in the database, it shows the
> >> Details panel (see the attached screenshot abrt-no-crashes-avail.png).
> >> The panel should be probably hidden, or it can show something like "No
> >> crash has been detected by ABRT yet." instead of the empty controls.
> >> Also, the buttons "Delete", "Copy to Clipboard", and
> "Report" should be
> >> disabled (Copy to Clipboard throws an exception, the others do nothing).
> >>
> >
> > - fixed
> >
> >> K.
>  _______________________________________________
>  Crash-catcher mailing list
>  Crash-catcher(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/crash-catcher