Merge into trunk : buttons__938167 : Code : Ubuntu Apps Directory

Reviewer	Date Requested	Status
Danny Tamez (community)		Approve on 2012-03-13
Michael Nelson (community)	2012-03-09	Approve on 2012-03-13
Review via email: mp+96836@code.staging.launchpad.net

Revision history for this message

Michael Nelson (michael.nelson) wrote on 2012-03-12:

#

On Fri, Mar 9, 2012 at 10:31 PM, Danny Tamez <email address hidden> wrote:
> Overview
> =========
> This branch adds fancy download buttons for apps an also has some drive by fixes for 2 related bugs.
>
> Details
> =========
> For bug 938167 - new buttons are being used and replace the old grey buttons. Here are some screenshots:
> Purchase button: http://simplest-image-hosting.net/png-0-ac2075
> Free button: http://simplest-image-hosting.net/png-0-gr2075
> Download ubuntu: http://simplest-image-hosting.net/png-0-wg2075

Cool - I like the purchase and free buttons. I'm not sure about the
download ubuntu button there though? It seems a bit out of place there
next to the app, and would perhaps belong elsewhere on the page (ie.
where there's space to say "Haven't tried Ubuntu? [[Download Ubuntu
graphic/link]]"). But if it's something you've already discussed in a
pre-imp. with someone, great.

I've put that comment on the bug so if there is a discussion, it can
happen there. I realise that's not something you'd changed (it's an
option to the template tag), but it may or may not be worth
discussing.

>
> For bug 861593 - the issue with bad wrapping and button display has been alleviated. Screenshots:
> Chrome button: http://simplest-image-hosting.net/png-0-io2075
> Chrome also available: http://simplest-image-hosting.net/png-0-jv2086
> Since this anchor was really a link to the download page and not a download itself I left this as a link and not a button.

Yikes - yes, +1 for that being a link. The link looks much more
natural. I've put a few comments with the inline changes related to
this.

>
> For bug 944254 - the text about the app not being available for your operating system has been removed.

I've added a comment on that bug too (we should use {% trans %} for
the title of the images).

Other comments inline below.

>
> To Test
> ========
> $fab bootstrap test
>
> --
> https://code.launchpad.net/~zematynnad/ubuntu-webcatalog/buttons__938167/+merge/96836
> You are subscribed to branch lp:ubuntu-webcatalog.
>
> === modified file 'src/webcatalog/static/css/webcatalog.css'
> --- src/webcatalog/static/css/webcatalog.css 2012-03-08 20:27:19 +0000
> +++ src/webcatalog/static/css/webcatalog.css 2012-03-09 21:30:35 +0000
> === added file 'src/webcatalog/static/images/scbutton-free-200px.png'
> Binary files src/webcatalog/static/images/scbutton-free-200px.png 1970-01-01 00:00:00 +0000 and src/webcatalog/static/images/scbutton-free-200px.png 2012-03-09 21:30:35 +0000 differ
> === added file 'src/webcatalog/static/images/scbutton-non-free-200px.png'
> Binary files src/webcatalog/static/images/scbutton-non-free-200px.png 1970-01-01 00:00:00 +0000 and src/webcatalog/static/images/scbutton-non-free-200px.png 2012-03-09 21:30:35 +0000 differ
> === modified file 'src/webcatalog/templates/webcatalog/install_options_snippet.html'
> --- src/webcatalog/templates/webcatalog/install_options_snippet.html 2011-09-19 21:33:34 +0000
> +++ src/webcatalog/templates/webcatalog/install_options_snippet.html 2012-03-09 21:30:35 +0000
> @@ -1,16 +1,20 @@
> {% load i18n %}
> {% if message_text %}
> - {{ message_text }}
> -{% endif %}{% if...

On Fri, Mar 9, 2012 at 10:31 PM, Danny Tamez <danny.tamez@canonical.com> wrote:
> Overview
> =========
> This branch adds fancy download buttons for apps an also has some drive by fixes for 2 related bugs.
>
> Details
> =========
> For bug 938167 - new buttons are being used and replace the old grey buttons.  Here are some screenshots:
> Purchase button: http://simplest-image-hosting.net/png-0-ac2075
> Free button: http://simplest-image-hosting.net/png-0-gr2075
> Download ubuntu: http://simplest-image-hosting.net/png-0-wg2075

Cool - I like the  purchase and free buttons. I'm not sure about the
download ubuntu button there though? It seems a bit out of place there
next to the app, and would perhaps belong elsewhere on the page (ie.
where there's space to say "Haven't tried Ubuntu? [[Download Ubuntu
graphic/link]]"). But if it's something you've already discussed in a
pre-imp. with someone, great.

I've put that comment on the bug so if there is a discussion, it can
happen there. I realise that's not something you'd changed (it's an
option to the template tag), but it may or may not be worth
discussing.

>
> For bug 861593 - the issue with bad wrapping and button display has been alleviated. Screenshots:
> Chrome button: http://simplest-image-hosting.net/png-0-io2075
> Chrome also available: http://simplest-image-hosting.net/png-0-jv2086
> Since this anchor was really a link to the download page and not a download itself I left this as a link and not a button.

Yikes - yes, +1 for that being a link. The link looks much more
natural. I've put a few comments with the inline changes related to
this.

>
> For bug 944254 - the text about the app not being available for your operating system has been removed.

I've added a comment on that bug too (we should use {% trans %} for
the title of the images).

Other comments inline below.

>
> To Test
> ========
> $fab bootstrap test
>
> --
> https://code.launchpad.net/~zematynnad/ubuntu-webcatalog/buttons__938167/+merge/96836
> You are subscribed to branch lp:ubuntu-webcatalog.
>
> === modified file 'src/webcatalog/static/css/webcatalog.css'
> --- src/webcatalog/static/css/webcatalog.css    2012-03-08 20:27:19 +0000
> +++ src/webcatalog/static/css/webcatalog.css    2012-03-09 21:30:35 +0000
> === added file 'src/webcatalog/static/images/scbutton-free-200px.png'
> Binary files src/webcatalog/static/images/scbutton-free-200px.png       1970-01-01 00:00:00 +0000 and src/webcatalog/static/images/scbutton-free-200px.png      2012-03-09 21:30:35 +0000 differ
> === added file 'src/webcatalog/static/images/scbutton-non-free-200px.png'
> Binary files src/webcatalog/static/images/scbutton-non-free-200px.png   1970-01-01 00:00:00 +0000 and src/webcatalog/static/images/scbutton-non-free-200px.png  2012-03-09 21:30:35 +0000 differ
> === modified file 'src/webcatalog/templates/webcatalog/install_options_snippet.html'
> --- src/webcatalog/templates/webcatalog/install_options_snippet.html    2011-09-19 21:33:34 +0000
> +++ src/webcatalog/templates/webcatalog/install_options_snippet.html    2012-03-09 21:30:35 +0000
> @@ -1,16 +1,20 @@
>  {% load i18n %}
>  {% if message_text %}
> -  {{ message_text }}
> -{% endif %}{% if display_install_button %}
> -  <a href="apt://{{ application.package_name }}" class="install-button">
> +    <div id="message_text">
> +        {{ message_text }}
> +    </div>
> +{% endif %}
> +{% if display_install_button %}
> +  <a href="apt://{{ application.package_name }}">
>   {% if application.for_purchase %}
> -  {% blocktrans with application.name as application_name %}Purchase {{ application_name }}{% endblocktrans %}
> +<img class="fancy-install" width="200" height="60" alt="" src="{{ STATIC_URL }}images/scbutton-non-free-200px.png" title="Get it from the Ubuntu Software Centre!">

We should use {% trans %} here for the title. And should the title
text say "Buy it.."?

>   {% else %}
> -  {% blocktrans with application.name as application_name %}Install {{ application_name }}{% endblocktrans %}
> +<img class="fancy-install" width="200" height="60" alt="" src="{{ STATIC_URL }}images/scbutton-free-200px.png" title="Get it from the Ubuntu Software Centre!">

Just {% trans %}

>   {% endif %}
>   </a>
>  {% endif %}
>  {% if display_ubuntu_download %}
> -<a href="http://www.ubuntu.com/download" class="awesome">{% trans "Download Ubuntu" %}</a>
> +<a href="http://www.ubuntu.com/download" class="awesome">
> +<img class="fancy-install" width="200" height="60" alt="" src="{{ STATIC_URL }}images/download_ubuntu.png" title="Get it from the Ubuntu Software Centre!">

{% trans %} and does this title make sense here?

> +</a>
>  {% endif %}
> -
>
> === modified file 'src/webcatalog/templatetags/webcatalog.py'
> --- src/webcatalog/templatetags/webcatalog.py   2012-03-01 21:38:31 +0000
> +++ src/webcatalog/templatetags/webcatalog.py   2012-03-09 21:30:35 +0000
> @@ -48,7 +48,8 @@
>     """Evaluate the installation options for the application in context."""
>     useragent = UserAgentString(context['user_agent'])
>     template_context = dict(application=application, message_text='',
> -        display_install_button=False, display_ubuntu_download=False)
> +        display_install_button=False, display_ubuntu_download=False,
> +                            STATIC_URL=settings.STATIC_URL)

It shouldn't be necessary to pass STATIC_URL in the context at all...
it smells like we're not using the static context processor? If that's
the case, let's update to use that instead.

https://docs.djangoproject.com/en/1.3/ref/contrib/staticfiles/#the-static-context-processor

>
>     if useragent.distroseries == application.distroseries.code_name:
>         # If both the distroseries and architecture matches, install away,
> @@ -104,9 +105,7 @@
>         template_context['display_install_button'] = True
>         return template_context
>     else:
> -        template_context['message_text'] = _(
> -            '{application_name} is not available for your operating '
> -            'system.').format(application_name=application.name)
> +        template_context['message_text'] = ''

See comment on bug about this.

>         template_context['display_ubuntu_download'] = True
>         return template_context
>
>
> === modified file 'src/webcatalog/tests/test_templatetags.py'
> --- src/webcatalog/tests/test_templatetags.py   2012-01-06 17:54:47 +0000
> +++ src/webcatalog/tests/test_templatetags.py   2012-03-09 21:30:35 +0000
> @@ -74,6 +74,7 @@
>
>         self.assertEqual(
>             dict(application=application, display_install_button=True,
> +                 STATIC_URL='/assets/',

Updating to use the static context processor should mean that all the
changes in this file can disappear.

>                  message_text='', display_ubuntu_download=False),
>             install_options(context, application))
>
> @@ -90,6 +91,7 @@
>         self.assertEqual(dict(
>             application=application, display_install_button=False,
>             message_text='Package Foo is not available for your computer.',
> +            STATIC_URL='/assets/',
>             display_ubuntu_download=False),
>             install_options(context, application))
>
> @@ -109,6 +111,7 @@
>         lucid_app_url = reverse('wc-package-detail', args=['lucid', 'pkgfoo'])
>         self.assertEqual(dict(
>             application=natty_application, display_install_button=False,
> +            STATIC_URL='/assets/',
>             message_text='Package Foo is also available for '
>                          '<a href="{lucid_app_url}">your version of Ubuntu'
>                          '</a>.'.format(lucid_app_url=lucid_app_url),
> @@ -128,6 +131,7 @@
>             application=natty_application, display_install_button=False,
>             message_text='Package Foo is not available for your version '
>                          'of Ubuntu.',
> +            STATIC_URL='/assets/',
>             display_ubuntu_download=False),
>             install_options(context, natty_application))
>
> @@ -144,7 +148,8 @@
>
>         self.assertEqual(
>             dict(application=application, display_install_button=True,
> -                 message_text='', display_ubuntu_download=False),
> +                 STATIC_URL='/assets/', message_text='',
> +                 display_ubuntu_download=False),
>             install_options(context, application))
>
>     def test_non_linux_displays_download_ubuntu(self):
> @@ -156,8 +161,8 @@
>
>         self.assertEqual(
>             dict(application=application, display_install_button=False,
> -                 message_text='Package Foo is not available for your '
> -                              'operating system.',
> +                 message_text='',
> +                 STATIC_URL='/assets/',
>                  display_ubuntu_download=True),
>             install_options(context, application))
>
>
> === modified file 'src/webcatalog/tests/test_views.py'
> --- src/webcatalog/tests/test_views.py  2012-03-08 21:50:03 +0000
> +++ src/webcatalog/tests/test_views.py  2012-03-09 21:30:35 +0000

-- 
Michael Nelson
Canonical Ltd.
+49 176 491 53481 (mob)
michael.nelson@canonical.com
IRC nick: noodles (noodles775 on Freenode)
Ubuntu - Linux for human beings | www.ubuntu.com | www.canonical.com

Revision history for this message

Danny Tamez (zematynnad) wrote on 2012-03-13:

#

Fixed the {{STATIC_URL}} issue. We were already using the static context processor but the templatetag was overwriting the RequestContext and thus the context processor values were no longer available. I changed the template tag to instead modify the RequestContext with the new values instead of creating an entirely new context.

As for the {{ trans }} issues: none of the application is translated and so we decided there's really no point in doing that.

Changed the tooltip on the non-free button to say 'Buy it' instead of 'Get it'.

We discussed the placement of the download/install button and decided it would probably look better beneath the description to the left of the screenshot or just underneath the screenshot. I left it underneath the description and here are screenies of each:
button below description: http://simplest-image-hosting.net/png-0-hj2158
button below screenshot: http://simplest-image-hosting.net/png-0-tn2158

Revision history for this message

Michael Nelson (michael.nelson) wrote on 2012-03-13:

#

Download full text (8.7 KiB)

Hey Danny,

I've just moved your replies inline so I can see what the context was...

On Mon, Mar 12, 2012 at 11:07 AM, Michael Nelson
<email address hidden> wrote:
> On Fri, Mar 9, 2012 at 10:31 PM, Danny Tamez <email address hidden> wrote:
>> Overview
>> =========
>> This branch adds fancy download buttons for apps an also has some drive by fixes for 2 related bugs.
>>
>> Details
>> =========
>> For bug 938167 - new buttons are being used and replace the old grey buttons. Here are some screenshots:
>> Purchase button: http://simplest-image-hosting.net/png-0-ac2075
>> Free button: http://simplest-image-hosting.net/png-0-gr2075
>> Download ubuntu: http://simplest-image-hosting.net/png-0-wg2075
>
> Cool - I like the purchase and free buttons. I'm not sure about the
> download ubuntu button there though? It seems a bit out of place there
> next to the app, and would perhaps belong elsewhere on the page (ie.
> where there's space to say "Haven't tried Ubuntu? [[Download Ubuntu
> graphic/link]]"). But if it's something you've already discussed in a
> pre-imp. with someone, great.
>
> I've put that comment on the bug so if there is a discussion, it can
> happen there. I realise that's not something you'd changed (it's an
> option to the template tag), but it may or may not be worth
> discussing.

On Tue, Mar 13, 2012 at 4:02 AM, Danny Tamez <email address hidden> wrote:
> We discussed the placement of the download/install button and decided it would probably look better beneath the description to the left of the screenshot or just underneath the screenshot. I left it underneath the description and here are screenies of each:
> button below description: http://simplest-image-hosting.net/png-0-hj2158
> button below screenshot: http://simplest-image-hosting.net/png-0-tn2158

If Anthony is happy with that placement, great. My comment above
wasn't about placement, in fact I thought the purchase and free
buttons looked great where they were (but I'm no designer). My comment
was just specifically about the "Download *Ubuntu*" button - it's not
something you've changed in this branch, just something I noticed that
it seems to lack any context (that is, as a non-Ubuntu user, I'd
expect something like "{{ app_name }} is available for Ubuntu. Haven't
tried Ubuntu? [[ Download Ubuntu graphic/link ]]"), which obviously
wouldn't fit up the top. But anyway, it's not a change of yours, just
something I thought while looking at the screenshots, so feel free to
ignore.

>
>>
>> For bug 861593 - the issue with bad wrapping and button display has been alleviated. Screenshots:
>> Chrome button: http://simplest-image-hosting.net/png-0-io2075
>> Chrome also available: http://simplest-image-hosting.net/png-0-jv2086
>> Since this anchor was really a link to the download page and not a download itself I left this as a link and not a button.
>
> Yikes - yes, +1 for that being a link. The link looks much more
> natural. I've put a few comments with the inline changes related to
> this.
>
>>
>> For bug 944254 - the text about the app not being available for your operating system has been removed.
>
> I've added a comment on that bug too (we should use {% ...

Hey Danny,

I've just moved your replies inline so I can see what the context was...

On Mon, Mar 12, 2012 at 11:07 AM, Michael Nelson
<michael.nelson@canonical.com> wrote:
> On Fri, Mar 9, 2012 at 10:31 PM, Danny Tamez <danny.tamez@canonical.com> wrote:
>> Overview
>> =========
>> This branch adds fancy download buttons for apps an also has some drive by fixes for 2 related bugs.
>>
>> Details
>> =========
>> For bug 938167 - new buttons are being used and replace the old grey buttons.  Here are some screenshots:
>> Purchase button: http://simplest-image-hosting.net/png-0-ac2075
>> Free button: http://simplest-image-hosting.net/png-0-gr2075
>> Download ubuntu: http://simplest-image-hosting.net/png-0-wg2075
>
> Cool - I like the  purchase and free buttons. I'm not sure about the
> download ubuntu button there though? It seems a bit out of place there
> next to the app, and would perhaps belong elsewhere on the page (ie.
> where there's space to say "Haven't tried Ubuntu? [[Download Ubuntu
> graphic/link]]"). But if it's something you've already discussed in a
> pre-imp. with someone, great.
>
> I've put that comment on the bug so if there is a discussion, it can
> happen there. I realise that's not something you'd changed (it's an
> option to the template tag), but it may or may not be worth
> discussing.

On Tue, Mar 13, 2012 at 4:02 AM, Danny Tamez <danny.tamez@canonical.com> wrote:
> We discussed the placement of the download/install button and decided it would probably look better beneath the description to the left of the screenshot or just underneath the screenshot.  I left it underneath the description and here are screenies of each:
> button below description: http://simplest-image-hosting.net/png-0-hj2158
> button below screenshot: http://simplest-image-hosting.net/png-0-tn2158

If Anthony is happy with that placement, great. My comment above
wasn't about placement, in fact I thought the purchase and free
buttons looked great where they were (but I'm no designer). My comment
was just specifically about the "Download *Ubuntu*" button - it's not
something you've changed in this branch, just something I noticed that
it seems to lack any context (that is, as a non-Ubuntu user, I'd
expect something like "{{ app_name }} is available for Ubuntu. Haven't
tried Ubuntu? [[ Download Ubuntu graphic/link ]]"), which obviously
wouldn't fit up the top. But anyway, it's not a change of yours, just
something I thought while looking at the screenshots, so feel free to
ignore.

>
>>
>> For bug 861593 - the issue with bad wrapping and button display has been alleviated. Screenshots:
>> Chrome button: http://simplest-image-hosting.net/png-0-io2075
>> Chrome also available: http://simplest-image-hosting.net/png-0-jv2086
>> Since this anchor was really a link to the download page and not a download itself I left this as a link and not a button.
>
> Yikes - yes, +1 for that being a link. The link looks much more
> natural. I've put a few comments with the inline changes related to
> this.
>
>>
>> For bug 944254 - the text about the app not being available for your operating system has been removed.
>
> I've added a comment on that bug too (we should use {% trans %} for
> the title of the images).
>
> Other comments inline below.
>
>>
>> To Test
>> ========
>> $fab bootstrap test
>>
>> --
>> https://code.launchpad.net/~zematynnad/ubuntu-webcatalog/buttons__938167/+merge/96836
>> You are subscribed to branch lp:ubuntu-webcatalog.
>>
>> === modified file 'src/webcatalog/templates/webcatalog/install_options_snippet.html'
>> --- src/webcatalog/templates/webcatalog/install_options_snippet.html    2011-09-19 21:33:34 +0000
>> +++ src/webcatalog/templates/webcatalog/install_options_snippet.html    2012-03-09 21:30:35 +0000
>> @@ -1,16 +1,20 @@
>>  {% load i18n %}
>>  {% if message_text %}
>> -  {{ message_text }}
>> -{% endif %}{% if display_install_button %}
>> -  <a href="apt://{{ application.package_name }}" class="install-button">
>> +    <div id="message_text">
>> +        {{ message_text }}
>> +    </div>
>> +{% endif %}
>> +{% if display_install_button %}
>> +  <a href="apt://{{ application.package_name }}">
>>   {% if application.for_purchase %}
>> -  {% blocktrans with application.name as application_name %}Purchase {{ application_name }}{% endblocktrans %}
>> +<img class="fancy-install" width="200" height="60" alt="" src="{{ STATIC_URL }}images/scbutton-non-free-200px.png" title="Get it from the Ubuntu Software Centre!">
>
> We should use {% trans %} here for the title. And should the title
> text say "Buy it.."?

On Tue, Mar 13, 2012 at 4:02 AM, Danny Tamez <danny.tamez@canonical.com> wrote:
> Changed the tooltip on the non-free button to say 'Buy it' instead of 'Get it'.

Great

> As for the {{ trans }} issues: none of the application is translated and so we decided there's really no point in doing that.

Are you sure you guys aren't getting confused with actual translations
for application data? That we're not supporting yet, sure, but AFAIK
we are currently ensuring that all the static text strings in our UI
(such as the title attribute above) are translateable? You can see
this in the above with the {% blocktrans %} that you've removed - the
"Purchase {{ applicatiion_name }}" was marked for translation, but
you've replaced it with an image title "Get it from the Ubuntu
Software Center!" which is not marked for translation.

Anyway, if that's all stuff you guys were aware of when you decided,
then that's fine. Maybe it's also worth chatting about after a
standup, as it's news to me and may be to others too.

>
>>   {% else %}
>> -  {% blocktrans with application.name as application_name %}Install {{ application_name }}{% endblocktrans %}
>> +<img class="fancy-install" width="200" height="60" alt="" src="{{ STATIC_URL }}images/scbutton-free-200px.png" title="Get it from the Ubuntu Software Centre!">
>
> Just {% trans %}
>
>>   {% endif %}
>>   </a>
>>  {% endif %}
>>  {% if display_ubuntu_download %}
>> -<a href="http://www.ubuntu.com/download" class="awesome">{% trans "Download Ubuntu" %}</a>
>> +<a href="http://www.ubuntu.com/download" class="awesome">
>> +<img class="fancy-install" width="200" height="60" alt="" src="{{ STATIC_URL }}images/download_ubuntu.png" title="Get it from the Ubuntu Software Centre!">
>
> {% trans %} and does this title make sense here?

I think you missed this question? (at least, I don't see it answered
or updated in your branch). That is, it's a "Download Ubuntu" button,
but the title text says "Get it from the Ubuntu Software Centre!"

>
>> +</a>
>>  {% endif %}
>> -
>>
>> === modified file 'src/webcatalog/templatetags/webcatalog.py'
>> --- src/webcatalog/templatetags/webcatalog.py   2012-03-01 21:38:31 +0000
>> +++ src/webcatalog/templatetags/webcatalog.py   2012-03-09 21:30:35 +0000
>> @@ -48,7 +48,8 @@
>>     """Evaluate the installation options for the application in context."""
>>     useragent = UserAgentString(context['user_agent'])
>>     template_context = dict(application=application, message_text='',
>> -        display_install_button=False, display_ubuntu_download=False)
>> +        display_install_button=False, display_ubuntu_download=False,
>> +                            STATIC_URL=settings.STATIC_URL)
>
> It shouldn't be necessary to pass STATIC_URL in the context at all...
> it smells like we're not using the static context processor? If that's
> the case, let's update to use that instead.
>
> https://docs.djangoproject.com/en/1.3/ref/contrib/staticfiles/#the-static-context-processor
>

On Tue, Mar 13, 2012 at 4:02 AM, Danny Tamez <danny.tamez@canonical.com> wrote:
> Fixed the {{STATIC_URL}} issue.  We were already using the static context processor but the
> templatetag was overwriting the RequestContext and thus the context processor values were no longer
> available.  I changed the template tag to instead modify the RequestContext with the new values instead
> of creating an entirely new context.
>

Great.

>>
>>     if useragent.distroseries == application.distroseries.code_name:
>>         # If both the distroseries and architecture matches, install away,
>> @@ -104,9 +105,7 @@
>>         template_context['display_install_button'] = True
>>         return template_context
>>     else:
>> -        template_context['message_text'] = _(
>> -            '{application_name} is not available for your operating '
>> -            'system.').format(application_name=application.name)
>> +        template_context['message_text'] = ''
>
> See comment on bug about this.

Let me know what you think about this. The comment on the bug will
give more context at:

https://bugs.launchpad.net/ubuntu-webcatalog/+bug/944254/comments/3

If you've already discussed it with someone, made a decision, and just
not updated the bug, no probs, otherwise let me know.

>
>>         template_context['display_ubuntu_download'] = True
>>         return template_context
>>
>>

Revision history for this message

Michael Nelson (michael.nelson) wrote on 2012-03-13:

#

On Tue, Mar 13, 2012 at 10:52 AM, Michael Nelson
<email address hidden> wrote:
>> {% trans %} and does this title make sense here?
>
> I think you missed this question? (at least, I don't see it answered
> or updated in your branch). That is, it's a "Download Ubuntu" button,
> but the title text says "Get it from the Ubuntu Software Centre!"

So that's being updated right now, and...
>
>>> if useragent.distroseries == application.distroseries.code_name:
>>> # If both the distroseries and architecture matches, install away,
>>> @@ -104,9 +105,7 @@
>>> template_context['display_install_button'] = True
>>> return template_context
>>> else:
>>> - template_context['message_text'] = _(
>>> - '{application_name} is not available for your operating '
>>> - 'system.').format(application_name=application.name)
>>> + template_context['message_text'] = ''
>>
>> See comment on bug about this.
>
> Let me know what you think about this. The comment on the bug will
> give more context at:
>
> https://bugs.launchpad.net/ubuntu-webcatalog/+bug/944254/comments/3
>
> If you've already discussed it with someone, made a decision, and just
> not updated the bug, no probs, otherwise let me know.

You mentioned that we're already doing the right thing in the other
case... great!

Thanks Danny

Revision history for this message

Michael Nelson (michael.nelson) on 2012-03-13:

#

review: Approve

Revision history for this message

ISD Branch Mangler (isd-branches-mangler) wrote on 2012-03-13:

#

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

Revision history for this message

Danny Tamez (zematynnad) wrote on 2012-03-13:

#

Already went over these changes with noodles/achuni

review: Approve

Ubuntu Apps Directory

Merge lp://staging/~zematynnad/ubuntu-webcatalog/buttons__938167 into lp://staging/ubuntu-webcatalog

Commit message

Description of the change

Preview Diff

Subscribers