Skip to content
This repository has been archived by the owner on Aug 21, 2023. It is now read-only.

Create and use HISTORIC_RANGE_DEFAULT in extra params components #15

Merged
merged 3 commits into from
Nov 22, 2017

Conversation

CloudNiner
Copy link
Contributor

Value of HISTORIC_RANGE_DEFAULT matches default historic_range
param value of the Climate API

Overview

Updates the historic range param controls to just automatically set themselves to the same value that the Climate API defaults to (1971) without presenting a "blank" option, since the API selects a default anyways if no param value is provided. This reduces confusion as to what the dropdown options mean in the Lab.

Demo

Both screenshots taken after selecting the indicator shown. No further actions taken to select extra params values:

screen shot 2017-11-20 at 15 48 23
screen shot 2017-11-20 at 15 48 14

Testing Instructions

Once this branch is checked out, run:

yarn run build:library && npm pack

Now checkout develop on climate-change-lab, ensure node_modules is up to date with yarn install and then manually install the climate-change-components package with:

npm install ../climate-change-components/climate-change-components-0.2.1.tgz

You should now be able to select any of the indicators that use the historic_range param and get a default value selected for you when the indicator loads.

I had some trouble with my apiHost in lab set to https://app.staging.climate.azavea.com. Starting my local Climate API box on at least the current develop and setting apiHost = http://localhost:8080 worked as a replacement.

Closes #12

Value of HISTORIC_RANGE_DEFAULT matches default historic_range
param value of the Climate API
Copy link
Contributor

@flibbertigibbet flibbertigibbet left a comment

Choose a reason for hiding this comment

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

Since the range defaults to the most recent base year and the base year options are known, it would be better to inspect historicRangeOptions and use the most recent year instead of hard-coding 1971.

@CloudNiner
Copy link
Contributor Author

CloudNiner commented Nov 22, 2017

Ok. I updated the PR to dynamically choose the default year. Unfortunately this has the side effect of making an additional pair of duplicate requests (although we can't always assume they're duplicates) just with the parameter added. I poked at seeing if I could clean up the request list quickly but its a broader problem across all indicators that this just makes worse for one case. I opened azavea/climate-change-lab#322 to address it.

this.historicRangeOptions.unshift('');
this.historicRangeOptions = data.map(h => parseInt(h.start_year, 10));
if (!this.extraParams.historic_range) {
const latestHistoricRange = Math.max(...this.historicRangeOptions);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ellipsis for an array builder, neat

Copy link
Contributor

@flibbertigibbet flibbertigibbet left a comment

Choose a reason for hiding this comment

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

+1 on update to CHANGELOG

@CloudNiner CloudNiner merged commit be22b7e into develop Nov 22, 2017
@CloudNiner CloudNiner deleted the feature/awf/historic-range-default#12 branch November 22, 2017 16:24
@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
- PercentileHistoricComponent and HistoricComponent now automatically choose a sensible dropdown
Copy link
Contributor

Choose a reason for hiding this comment

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

Would have preferred something more specific than "sensible", i.e., explicitly match the default from the API.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants