Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ecobenefits server template #92

Merged
merged 6 commits into from
Jul 26, 2013
Merged

Ecobenefits server template #92

merged 6 commits into from
Jul 26, 2013

Conversation

RickMohr
Copy link
Contributor

Apply eco-benefits template server-side instead of client-side

<div>
<h2>Benefits</h2>
<table>
<tr><th>Benefit</th><th>Value</th></tr>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the the first bits of template markup that we are probably not going to throw out once we get full prototype markup from the designers. As a result, we should consider replacing these literal English string with internationalized versions.

I would suggest modifying _benefits_for_trees in ecobenefits/views.py to include a translated label for each benefit. A Spanish tree map would return something like this:

{
  ...
  "airquality": {
    value: 12
    unit: "kg"
    label: "Calidad del aire"
  }
  ...
}

This template can then be generalized to

<tr><td>{{ benefits.airquality.label }}<td>{{ benefits.airquality.value }}</td></tr>

and then simplified further into a loop that iterates over the keys in benefits

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that. My inclination for this pull request is to add "label" to the
benefit hash and add an i18n card to the backlog. You and Adam can decide
if we should promote it to this sprint.

A related issue is units. For each instance, how will we choose units for
e.g. the "air quality" benefit or for a tree's diameter:

  1. Based on the instance's locale, or
  2. Allow each instance to configure units separately for each of many
    fields, or
  3. Add in basic auditing proof of concept #1 by default with Topic/vagrant #2 optional
  4. Something else

Whatever the answer, we should add a card for setting up its plumbing. And
one for updating the benefits display to use appropriate units. For now
I'll change the benefits template to actually display the units.

-Rick

On Thu, Jul 25, 2013 at 3:40 PM, Justin Walgran notifications@github.comwrote:

In opentreemap/treemap/templates/treemap/eco_benefits.html:

@@ -0,0 +1,13 @@
+

  • Benefits

  • This is one of the the first bits of template markup that we are probably
    not going to throw out once we get full prototype markup from the
    designers. As a result, we should consider replacing these literal English
    string with internationalized versionshttps://docs.djangoproject.com/en/dev/topics/i18n/translation/
    .

    I would suggest modifying _benefits_for_trees in ecobenefits/views.py to
    include a translated label for each benefit. A Spanish tree map would
    return something like this:

    {
    ...
    "airquality": {
    value: 12
    unit: "kg"
    label: "Calidad del aire"
    }
    ...}

    This template can then be generalized to

    and then simplified further into a loop that iterates over the keys in
    benefits


    Reply to this email directly or view it on GitHubhttps://github.com//pull/92/files#r5407842
    .

    Rick Mohr, Software Developer

    Azavea | 340 N 12th St, Ste 402, Philadelphia, PA
    rmohr@azavea.com | T 215.701.7504 | F 215.925.2663
    Web azavea.com http://www.azavea.com/ | Blog
    azavea.com/blogshttp://www.azavea.com/Blogs
    | Twitter @azavea http://twitter.com/azavea

    BenefitValue
    {{ benefits.airquality.label }}{{ benefits.airquality.value }}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same way that the locale setting will control the content of the label I think it should also control how the raw value that comes from eco.py should be converted into the content of the value and unit fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for MVP we'll say each instance specifies a locale, and that locale
determines the units used for all fields. In the future if some customer
beats on us we'll consider allowing field-specific overrides.

On Thu, Jul 25, 2013 at 5:03 PM, Justin Walgran notifications@github.comwrote:

In opentreemap/treemap/templates/treemap/eco_benefits.html:

@@ -0,0 +1,13 @@
+

  • Benefits

  • The same way that the locale setting will control the content of the labelI think it should also control how the raw value that comes from eco.py
    should be converted into the content of the value and unit fields.


    Reply to this email directly or view it on GitHubhttps://github.com//pull/92/files#r5409926
    .

    Rick Mohr, Software Developer

    Azavea | 340 N 12th St, Ste 402, Philadelphia, PA
    rmohr@azavea.com | T 215.701.7504 | F 215.925.2663
    Web azavea.com http://www.azavea.com/ | Blog
    azavea.com/blogshttp://www.azavea.com/Blogs
    | Twitter @azavea http://twitter.com/azavea

    BenefitValue

@jwalgran
Copy link
Contributor

+1

* Add label on each benefit (yet to be internationalized)
* Add unit on each benefit (yet to be locale-sensitive)
* Add number format string on each benefit
* Simplify template to use a loop
Rather than testing returned eco-benefits values, test that:
* trees without diameter, species, or itree code are ignored for benefits calculations
* extrapolated benefits are larger than non-extrapolated benefits

Also, simplify extrapolation calculation and reformat its code a bit
RickMohr added a commit that referenced this pull request Jul 26, 2013
@RickMohr RickMohr merged commit ce8e0f9 into OpenTreeMap:master Jul 26, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants