<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - Make demo extensible for other type of annotations"
href="https://bugs.freedesktop.org/show_bug.cgi?id=69978#c15">Comment # 15</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - Make demo extensible for other type of annotations"
href="https://bugs.freedesktop.org/show_bug.cgi?id=69978">bug 69978</a>
from <span class="vcard"><a class="email" href="mailto:gpoo@gnome.org" title="Germán Poo-Caamaño <gpoo@gnome.org>"> <span class="fn">Germán Poo-Caamaño</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=69978#c13">comment #13</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=86867" name="attach_86867" title="glib: Prepare UI to add multiple annotations type in demo">attachment 86867</a> <a href="attachment.cgi?id=86867&action=edit" title="glib: Prepare UI to add multiple annotations type in demo">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=69978&attachment=86867'>[review]</a> [review]
> glib: Prepare UI to add multiple annotations type in demo
>
> Review of <span class=""><a href="attachment.cgi?id=86867" name="attach_86867" title="glib: Prepare UI to add multiple annotations type in demo">attachment 86867</a> <a href="attachment.cgi?id=86867&action=edit" title="glib: Prepare UI to add multiple annotations type in demo">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=69978&attachment=86867'>[review]</a> [review]:
> -----------------------------------------------------------------
>
> ::: glib/demo/annots.c
> @@ +38,5 @@
> >
> > +enum {
> > + ANNOTS_NONE,
> > + ANNOTS_TEXT
> > +};
>
> Why not using PopplerAnnotType?
>
> @@ +52,4 @@
> > GtkWidget *timer_label;
> >
> > gint num_page;
> > + gint type_selector;
>
> type_selector sounds like the widget not the value. I would use
> selected_type or new_annot_type. I don't see why we need to save the value
> if we are using a modal dialgo to add annots.</span >
To make it extensible immediately. I could squash this changes when
adding the support for Lines (or another annotation type), but I think
it is cleaner to have them in separated commits, as in:
<a href="https://github.com/gpoo/poppler/commit/b9a3c2a2501169b2f7dafd868b9302a228b0e8cd">https://github.com/gpoo/poppler/commit/b9a3c2a2501169b2f7dafd868b9302a228b0e8cd</a>
[...]
<span class="quote">> @@ +973,5 @@
> >
> > gtk_box_pack_start (GTK_BOX (vbox), hbox, FALSE, TRUE, 0);
> > +
> > + type_selector = gtk_combo_box_text_new ();
> > + gtk_combo_box_text_append_text (GTK_COMBO_BOX_TEXT (type_selector), "None");
>
> I don't remember why we allowed to add unknown annots, but I think it
> doesn't make sense</span >
It does when the annotations are added interactively instead of a modal dialog
(in another patch submitted). This allow to add a new annotation just by
clicking or selecting a rectangle in the drawing area.
I could have removed it and added it later squashed in the another patch,
but I followed the same rationale of having separated commits.
I also could have added a toggle button or something like that, but I thought
it
was overkill at this stage. What do you thinl</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>