Code review comment for lp:~dobey/ubuntu-printing-app/add-notifier-daemon

Revision history for this message
dobey (dobey) wrote :

On Fri, 2017-03-17 at 15:56 +0000, Andrew Hayzen wrote:
> Review: Needs Fixing
>
> @dobey, would you mind doing a pull of trunk to bring in the new
> changes?

I'll get it updated.

> Also when trying to get this to build in a silo I got "No package
> 'properties-cpp' found", which suggests the package libproperties-
> cpp-dev should be added to the build-deps?

I'll see what's going on there.

> Furthermore when discussing with Bill we have a few changes (these
> can be done in this branch or a further branch):
> 1) Currently the text in the notification doesn't wrap, so you only
> see the first part. Both the design specifications show this (#1.2 in
> [0] and in [1]). Is this a limitation of the notifications in unity8?
> or an issue in this notifier code?

This would be an issue on the renderer's side. We have no way to know
what the width of the notification will be on the client (notifier
daemon) side, so we can't do anything about it there. It seems there is
no way to specify how this should be handled, from the client side.

> 2) We don't think there is an API yet for adding an entry for the
> printer queue into the launcher of unity8 when something is printing
> without launching an app (what design wants). So for now can you show
> a notification when something is sent to the printer ("Printing on
> printerName"), as shown in the design #1.1 in [0]

We can't really do this from notifier daemon which listens to cups for
status, no. It would really need to be a notification sent by the
printing app I think, but in 99.9999% of cases will be immediately
followed by the '"foo" has printed." notification. So there's no good
reason to pop a '"foo" is printing." notification immediately prior to.

> 3) If the user clicks on the notification when something is sent to
> the printer, this should show the queue dialog. I'm currently
> building the queue dialog in another branch, not decided on the
> launcher name yet - will let you know (probably something like
> ubuntu-queue-dialog, or maybe we could have a url-handler call for
> it?).

There is no way to implement this as "when the user clicks on the
notification." The design is wrong. To do this would require making the
notification interactive instead, and adding a "View print queue"
button to it.

However, looking at the doc, this is only specified for errors, and
it's specified to go to the printer's page in settings. The notifier
already adds a button to open settings on printer errors, which goes to
settings:///system/printers/${printer_name}. If that's already handled
in system-settings, then I think this is already implemented.

« Back to merge proposal