Skip to content
This repository has been archived by the owner. It is now read-only.

Port cookie functions from govuk_template #85

Merged
merged 2 commits into from Aug 24, 2015
Merged

Port cookie functions from govuk_template #85

merged 2 commits into from Aug 24, 2015

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Aug 20, 2015

  • Allow admin apps to easily set, retrieve and delete cookies.
  • Add option to specify the cookie’s domain (eg per app, or across apps)

Two immediate use cases will be:

fofr added 2 commits Aug 20, 2015
* Cookies may want to persist between admin apps
* Defaults to working within just the current app
@fofr fofr changed the title Port cookies function from govuk_template Port cookie functions from govuk_template Aug 20, 2015
@edds
Copy link
Contributor

@edds edds commented Aug 21, 2015

I can't help but feel it would be better to just include govuk_frontend_toolkit here rather than copy pasting the code. That way everything will benefit from any enhancements or bug fixes which make it into this code.

@robinwhittleton
Copy link

@robinwhittleton robinwhittleton commented Aug 21, 2015

Was going to suggest similar or just including cookie-functions.js a separate file.

@fofr
Copy link
Contributor Author

@fofr fofr commented Aug 21, 2015

@edds The cookie functions are in govuk_template, not the govuk_frontend_toolkit.

@fofr
Copy link
Contributor Author

@fofr fofr commented Aug 21, 2015

As this isn't something that's in govuk_frontend_toolkit, and govuk_template isn't an appropriate thing to include, are you happy to merge @edds?

edds added a commit that referenced this pull request Aug 24, 2015
Port cookie functions from govuk_template
@edds edds merged commit 6de57b6 into master Aug 24, 2015
1 check passed
1 check passed
default "Build #126 succeeded on Jenkins"
Details
@edds edds deleted the cookie-helpers branch Aug 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.