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

Auto-update signature count on petition page. #482

Closed
wants to merge 1 commit into from

Conversation

dracos
Copy link
Contributor

@dracos dracos commented Jun 25, 2016

This fetches the JSON of the petition every 10 seconds and updates the count, and progress bar, accordingly, with a pretty counting animation.

@dracos dracos force-pushed the auto-update-signature-count branch 2 times, most recently from 188601b to 3ac2ddd Compare June 26, 2016 15:46
@@ -23,6 +23,11 @@ def home_page?
params.values_at(:controller, :action) == %w[pages index]
end

def open_petition_page?
# XXX What should go here? Same as petition_page? plus petition.open? Is that allowed here?
true
Copy link
Contributor

Choose a reason for hiding this comment

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

The method petition_page? is just a check for the controller and action name but we know that if it's true then the @petition instance variable is set so just use the Petition#open? method to achieve what you need, e.g:

petition_page? && @petition.open?

Hopefully that's clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, great, thanks, I still don't get my head around where variables are magically available in ruby :)

@pixeltrix
Copy link
Contributor

The reason that archived_petition_page isn't in the fragment.yml file is that the head section isn't dependent on that - it's got :url in the list so would change anyway.

This also updates the progress bar.
@dracos dracos force-pushed the auto-update-signature-count branch from 3ac2ddd to 106f77f Compare June 28, 2016 12:29
@dracos
Copy link
Contributor Author

dracos commented Jun 28, 2016

Ah, of course, makes sense, thanks. Think only thing I don't understand now is the Keys class in CacheHelper, but I don't really need to to tidy this up ;) Fixes pushed.

@pixeltrix
Copy link
Contributor

Think only thing I don't understand now is the Keys class in CacheHelper

That's just a simple decorator around the template context so that we can return the values in the way that we need. Mostly it's simple delegations but some are slightly more complicated. It then creates the cache key for each item in the list of dependencies.

@pixeltrix
Copy link
Contributor

@dracos one more thing - I think creating a signature count API would be useful - there's no need to pull all of the JSON data just to get the signature count.

@pixeltrix
Copy link
Contributor

Closing in favour of the fixed #512 - thanks @dracos

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