Bug 70982 - glib-demo: Simplify the list view for annotations
Summary: glib-demo: Simplify the list view for annotations
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: glib frontend (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 70901 71727
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-29 07:15 UTC by Germán Poo-Caamaño
Modified: 2013-11-22 07:44 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
glib-demo: Simplify annotations list and its properties (5.50 KB, patch)
2013-10-29 07:15 UTC, Germán Poo-Caamaño
Details | Splinter Review
glib-demo: Simplify annotations list and its properties (5.42 KB, patch)
2013-11-05 19:37 UTC, Germán Poo-Caamaño
Details | Splinter Review
glib-demo: Simplify annotations list and its properties (5.55 KB, patch)
2013-11-07 02:01 UTC, Germán Poo-Caamaño
Details | Splinter Review
glib-demo: Simplify annotations list and its properties (5.42 KB, patch)
2013-11-16 08:13 UTC, Germán Poo-Caamaño
Details | Splinter Review
glib-demo: Simplify annotations list and its properties (5.40 KB, patch)
2013-11-18 09:30 UTC, Germán Poo-Caamaño
Details | Splinter Review
glib-demo: Simplify annotations list and its properties (6.90 KB, patch)
2013-11-19 22:04 UTC, Germán Poo-Caamaño
Details | Splinter Review

Description Germán Poo-Caamaño 2013-10-29 07:15:43 UTC
Created attachment 88269 [details] [review]
glib-demo: Simplify annotations list and its properties

glib-demo: Simplify annotations list and its properties

* Move the annotation's rectangle to properties to make the
  list view of annotations cleaner.
* Remove duplicated information: flags and page are already
  present in the UI.
Comment 1 Germán Poo-Caamaño 2013-11-05 19:37:48 UTC
Created attachment 88729 [details] [review]
glib-demo: Simplify annotations list and its properties

Updated patch to match with changes in https://bugs.freedesktop.org/attachment.cgi?id=88161 (https://bugs.freedesktop.org/show_bug.cgi?id=70901)
Comment 2 Germán Poo-Caamaño 2013-11-07 02:01:13 UTC
Created attachment 88801 [details] [review]
glib-demo: Simplify annotations list and its properties
Comment 3 Germán Poo-Caamaño 2013-11-16 08:13:10 UTC
Created attachment 89306 [details] [review]
glib-demo: Simplify annotations list and its properties

Updated patch
Comment 4 Germán Poo-Caamaño 2013-11-18 09:30:48 UTC
Created attachment 89392 [details] [review]
glib-demo: Simplify annotations list and its properties

Updated patch. Rebased to make it fit against patches in
https://bugs.freedesktop.org/show_bug.cgi?id=70981
Comment 5 Carlos Garcia Campos 2013-11-19 17:23:41 UTC
Comment on attachment 89392 [details] [review]
glib-demo: Simplify annotations list and its properties

Review of attachment 89392 [details] [review]:
-----------------------------------------------------------------

I'm not sure about this, I find it a bit confusing. This is what I see with my spanish locale:

Coords: (152,0,556,0) (152,0,556,0)

Maybe we could use something like:

Coords: X1: aa,bb X2: aa,bb Y1: aa,bb Y2: aa,bb

Which is what we currently have in the list but moved to a label in the properties.
The coords information is mainly useful when debugging, but now that we have the view also in the annots demo, it would be really useful to paint a focus ring around the currently selected annot. But that could be done as a follow up patch.
Comment 6 Germán Poo-Caamaño 2013-11-19 18:02:12 UTC
(In reply to comment #5)
> Comment on attachment 89392 [details] [review] [review]
> glib-demo: Simplify annotations list and its properties
> 
> Review of attachment 89392 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> I'm not sure about this, I find it a bit confusing. This is what I see with
> my spanish locale:
> 
> Coords: (152,0,556,0) (152,0,556,0)
> 
> Maybe we could use something like:
> 
> Coords: X1: aa,bb X2: aa,bb Y1: aa,bb Y2: aa,bb
> 
> Which is what we currently have in the list but moved to a label in the
> properties.

I was trying to save space, so no need to resize the sidebar pane and leaving space to the rendering page.  I had forgotten about the locale thing, though.

Maybe we can use semicolon instead a comma, something like:
     (152,0;556,0) (152,0;556,0)
or a dash:
     (152,0-556,0) (152,0-556,0)

FWIW, I was also inclined to removed the decimals, but the PDF has decimals (for the annotations added with other software). The ones created with cairo, are integers.

> The coords information is mainly useful when debugging, but now that we have
> the view also in the annots demo, it would be really useful to paint a focus
> ring around the currently selected annot. But that could be done as a follow
> up patch.

Indeed, the reason to save space and avoid the scrollbars in the side pane was precisely because it was becoming a pain to debug the coordinates when I was testing it :-)

The focus ring would be good. IIRC, I had it done in an Evince patch I have.
Comment 7 Carlos Garcia Campos 2013-11-19 18:08:17 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Comment on attachment 89392 [details] [review] [review] [review]
> > glib-demo: Simplify annotations list and its properties
> > 
> > Review of attachment 89392 [details] [review] [review] [review]:
> > -----------------------------------------------------------------
> > 
> > I'm not sure about this, I find it a bit confusing. This is what I see with
> > my spanish locale:
> > 
> > Coords: (152,0,556,0) (152,0,556,0)
> > 
> > Maybe we could use something like:
> > 
> > Coords: X1: aa,bb X2: aa,bb Y1: aa,bb Y2: aa,bb
> > 
> > Which is what we currently have in the list but moved to a label in the
> > properties.
> 
> I was trying to save space, so no need to resize the sidebar pane and
> leaving space to the rendering page.  I had forgotten about the locale
> thing, though.
> 
> Maybe we can use semicolon instead a comma, something like:
>      (152,0;556,0) (152,0;556,0)
> or a dash:
>      (152,0-556,0) (152,0-556,0)
> 
> FWIW, I was also inclined to removed the decimals, but the PDF has decimals
> (for the annotations added with other software). The ones created with
> cairo, are integers.

Yes, I prefer to keep two decimals, fwiw

> > The coords information is mainly useful when debugging, but now that we have
> > the view also in the annots demo, it would be really useful to paint a focus
> > ring around the currently selected annot. But that could be done as a follow
> > up patch.
> 
> Indeed, the reason to save space and avoid the scrollbars in the side pane
> was precisely because it was becoming a pain to debug the coordinates when I
> was testing it :-)
> 
> The focus ring would be good. IIRC, I had it done in an Evince patch I have.

Current evince already does that.
Comment 8 Germán Poo-Caamaño 2013-11-19 22:04:58 UTC
Created attachment 89498 [details] [review]
glib-demo: Simplify annotations list and its properties

Updated patch.

This patch also makes a fixed room for annotation properties.  When the annotation list is long, the properties jump ups and down.

I changed to 2 decimals and semicolon.

Something I forgot to mention: the format (x,y) is also more convenient for quadrilaterals (which can be a lot), so it helps to get a quicker idea of their coordinates.  However, those are not seen up to this patch yet.

What do you think?
Comment 9 Carlos Garcia Campos 2013-11-22 07:44:29 UTC
Comment on attachment 89498 [details] [review]
glib-demo: Simplify annotations list and its properties

Review of attachment 89498 [details] [review]:
-----------------------------------------------------------------

Ok, pushed, thanks!


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.