Skip to content

Issue/1682 - #2440

Merged
xsrust merged 3 commits into
developmentfrom
issue/1682
Mar 26, 2020
Merged

Issue/1682#2440
xsrust merged 3 commits into
developmentfrom
issue/1682

Conversation

@xsrust

@xsrust xsrust commented Mar 26, 2020

Copy link
Copy Markdown
Contributor
  • refactor the body of the accordions into a separate partial
  • Conditionally call the accordions or just the body based on whether the user's org is a funder
  • Made the JS more robust against the chart-data not existing on the page
  • Remove the 'Monthly Usage' download for funders, and add in some spacing to make the top buttons align if removed.

@briri briri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good @xsrust

@@ -1,4 +1,4 @@
import { isObject } from '../../utils/isType';
import { isObject, isUndefined } from '../../utils/isType';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 I probably should have been using this one!

const usersData = JSON.parse($('#users_joined').val());
if (isObject(usersData)) {
createChart('#yearly_users', usersData);
if (!isUndefined($('#users_joined').val())) {

@briri briri Mar 26, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good. I hope these checks become unnecessary in future versions of Rails' asset pipeline. It would be better if it just sent the client browser the JS it actually needs for the page.

@xsrust
xsrust merged commit 003431d into development Mar 26, 2020
@briri
briri deleted the issue/1682 branch April 28, 2020 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants