Opened 8 years ago

Closed 8 years ago

#533 closed task (fixed)

Simplify/Robustify thumbnail usage for template designers

Reported by: spaetz Owned by: spaetz
Priority: major Milestone: 0.3.2
Component: programming Keywords:
Cc: Parent Tickets:

Description (last modified by Christopher Allan Webber)

This is pre-work for the PDF media type handling which is supposed to show default pdf fallback icons in the beginning.

Please consider merging the branch get_thumb_url from my git repo at git://gitorious.org/~spaetz/mediagoblin/spaetz-mediagoblin.git.

Description of my change straight from the commit log:

    Simplify/Robustify the thumbnail URL usage in templates
    
    So far templates required a very complex blurb to simply insert a
    thumbnail URL, exposing much of the internal logic to the template
    designer. In addition, we would fail with an error if for some
    reason the media_files['thumb'] entry was never populated.
    
    This adds the MediaEntry.thumb_url property that template designers
    can simply use. It will do the right thing, either fetching the proper
    thumbnail or hand back a generic icon specified in a media's
    MEDIA_MANAGER as "default_thumb".
    
    Add an image default fallback icon (stolen from Tangos, which are
    Public Domain since version 0.8.90 as I understand) since the one
    we referred to was not existing. Perhaps, a "broken image" icon
    would be better, but I'll leave that to our capable designers.
    
    All templates have been modified to make use of the new thumb_url
    function.

Subtickets

Change History (5)

comment:1 Changed 8 years ago by spaetz

Milestone: 0.3.3

comment:2 Changed 8 years ago by Christopher Allan Webber

Description: modified (diff)

comment:3 Changed 8 years ago by Christopher Allan Webber

Heya Spaetz,

This looks really good. Only a couple of comments:

  • I'm always hesitant to use try: except: for things that aren't handling errors like checking for a key, maybe change to like:
      if self.media_files.has_key('thumb'):
        thumb_url = [foo]
      else:
        thumb_url = [bar]
    
  • For that matter, I wonder... what if the media manager has no fallback thumbnail either? maybe we should have a fallback-fallback thumbnail? so in the else: above have another if/else that checks to see if the media type provides such a thing; if so, use it, otherwise, use the generic media type one.

How's that? With that resolved I think this is good to merge.

Last edited 8 years ago by Christopher Allan Webber (previous) (diff)

comment:4 Changed 8 years ago by Elrond

Milestone: 0.3.30.3.2
Owner: set to spaetz
Status: newassigned

I think, I merged this a few weeks ago as 286478711728. And it was later enhanced to use the media manager more consistently (5f8b4ae8).

Can someone please confirm this?

comment:5 Changed 8 years ago by spaetz

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.