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

[Issue #12] Adding configuration to exclude manifest in template #28

Closed
wants to merge 2 commits into from

Conversation

safwanrahman
Copy link
Owner

It will add another configuration option from the settings. In the WEBPUSH_SETTINGS dictionary, just add MANIFEST: False and it will not include the manifest to the template. Need to add documentation for it.
@arogachev Can you please check if its working fine for you?

@arogachev
Copy link
Contributor

@safwanrahman What do you think about #12 (comment)? I think adding it manually regardless of existing manifest.json file is not a big deal.

@safwanrahman
Copy link
Owner Author

@arogachev I think, most of the people does not have already exist manifest.json. So adding it manually will be hassle to them. I think its a better way to do that.
What do you think?

@arogachev
Copy link
Contributor

arogachev commented Nov 11, 2016

@safwanrahman I don't see a hassle in adding it, especially if only one setting needs to be added and clear instructions are written. MANIFEST: False is overcomplication in my opinion.

@safwanrahman
Copy link
Owner Author

@arogachev I have discussed with someone and it seems that, not every django application does not have a core app where to include the manifest.json and in different project the core app name differs. So there are no exact solution to write a instruction about it. What will the user do if he does not have any core app? How we can write clear information about making a manifest.json and store it somewhere in the static directory?

@arogachev
Copy link
Contributor

@safwanrahman I think this should not be a problem. We can mention "core" as an example of a place where common code is located. Hovewer this is just my opinion, yours can differ and I can't force you to use my solution. Feel free to use your solution instead.

@safwanrahman
Copy link
Owner Author

@arogachev I have found a way to keep your suggestion as well as mine. I will update the PR today or tomorrow to reflect the changes

@safwanrahman
Copy link
Owner Author

@arogachev I have updated the patch. So now if anyone want to include the manifest, he should add webpush_manifest in the head of the template.
If you agree, I can add documentation for it.

@safwanrahman
Copy link
Owner Author

manifest.json is no longer needed in webpush for chrome. @arogachev Thanks a lot for the issue and discussion

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