[Bugs] #4384 Browse UNSP: Browse: display a message while downloading a PDF

[Sugar Labs Bugs]

#4384: Browse: display a message while downloading a PDF
------------------------------------------+---------------------------------
    Reporter:  manuq                      |          Owner:  manuq      
        Type:  enhancement                |         Status:  new        
    Priority:  Unspecified by Maintainer  |      Milestone:  1.0        
   Component:  Browse                     |        Version:  Unspecified
    Severity:  Unspecified                |       Keywords:  r?         
Distribution:  Unspecified                |   Status_field:  Unconfirmed
------------------------------------------+---------------------------------

Comment(by humitos):

 Replying to [comment:1 manuq]:
 > CC'ing Gary for design review, humitos for Browse review, erikos for
 toolkit review.

 Wow! This feature looks awesome for me. I like the style, the idea, and I
 think the user experience is much better now... I'm happy :)

 I have some minor comments about your patches:

  * sugar-toolkit-gtk3: set the default value for direction in the argument
 instead of using None and then check if that argument is None:
 {{{
 diff --git a/src/sugar3/graphics/iconprogress.py
 b/src/sugar3/graphics/iconprogress.py
 index 9f1c5a4..40e2712 100644
 --- a/src/sugar3/graphics/iconprogress.py
 +++ b/src/sugar3/graphics/iconprogress.py
 @@ -34,12 +34,10 @@ class IconProgress(Gtk.DrawingArea):

      """
      def __init__(self, icon_name, pixel_size, stroke_color, fill_color,
 -                 direction=None):
 +                 direction='vertical'):
          Gtk.DrawingArea.__init__(self)

          self._icon_name = icon_name
 -        if direction == None:
 -            direction = 'vertical'
          self._direction = direction
          self._progress = 0
 }}}

  * browse: I don't like (personal opinion) the use of
 {{{self.get_children()[0]}}} because if later we add a new child to
 PDFTabPage maybe the PDFMessageBox is not in the [0] position anymore. I
 prefer using {{{self.message_box = PDFMessageBox}}}.
 {{{
 diff --git a/pdfviewer.py b/pdfviewer.py
 index a3ededa..bd52bba 100644
 --- a/pdfviewer.py
 +++ b/pdfviewer.py
 @@ -408,11 +408,11 @@ class PDFTabPage(Gtk.HBox):
          """Download the PDF from a remote location to a temporal file."""

          # Display a message
 -        message_box = PDFMessageBox(
 +        self.message_box = PDFMessageBox(
              message=_("Downloading document..."),
              button_callback=self.cancel_download)
 -        self.pack_start(message_box, True, True, 0)
 -        message_box.show()
 +        self.pack_start(self.message_box, True, True, 0)
 +        self.message_box.show()

          # Figure out download URI
          temp_path = os.path.join(activity.get_activity_root(),
 'instance')
 @@ -438,8 +438,7 @@ class PDFTabPage(Gtk.HBox):
          self._browser.props.progress = progress

          # Update progress in the message box too
 -        message_box = self.get_children()[0]
 -        message_box.icon_progress.update(progress)
 +        self.message_box.icon_progress.update(progress)

      def __download_status_cb(self, download, data):
          status = download.get_status()
 @@ -449,8 +448,7 @@ class PDFTabPage(Gtk.HBox):
              self._browser.props.load_status = WebKit.LoadStatus.FINISHED

              # Remove the message box
 -            message_box = self.get_children()[0]
 -            self.remove(message_box)
 +            self.remove(self.message_box)

              self._show_pdf()
          elif status == WebKit.DownloadStatus.CANCELLED:
 }}}

 Besides these little comments, the patches looks good for me. #4385
 happened to me many times while I was testing this. Testing this patch
 with a fresh instance of Browse works perfect.

-- 
Ticket URL: <http://bugs.sugarlabs.org/ticket/4384#comment:3>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system