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

Fixes #87 #137

Merged
merged 5 commits into from Feb 28, 2019
Merged

Fixes #87 #137

merged 5 commits into from Feb 28, 2019

Conversation

pasevin
Copy link
Contributor

@pasevin pasevin commented Feb 24, 2019

This is my attempt at fixing this long-standing issue. It works for my needs without issues so far.
I've utilized MutationObserver for observing DOM node additions. And replaced Django render method with a javascript initialization instead.

@dominicrodger
Copy link
Contributor

This looks good to me - just two comments:

  1. Can you remove the imports of safestring, get_language and simplejson as json, since they're no longer used? (That'll fix the flake8 checks)
  2. My JavaScript knowledge isn't good enough to know what browsers this will work in - do you know? From a quick glance, I think MutationObserver is the main thing here, but that looks to be fairly widely supported. Anything else I'm missing?

Thanks for your efforts on this - they're much appreciated!

@pasevin
Copy link
Contributor Author

pasevin commented Feb 25, 2019

Cool!

  1. Done.
  2. I went with MutationObserver vs DOMNodeInserted, which is used more widely and easier to implement, but looking forward is deprecated. I think this is the only reasonable way to go.

@dominicrodger
Copy link
Contributor

Fantastic - thanks @pasevin! All going well I'll test this and merge it tonight.

@dominicrodger
Copy link
Contributor

Hi @pasevin - I can't get this to work locally - targetNode on line 7 of recurrent-widget.init.js is always null. Where does the class grp-content-container come from? I can't see it anywhere in Django's source. I've tested Django 2.1.7 and 1.11.14. Any ideas?

@pasevin
Copy link
Contributor Author

pasevin commented Feb 28, 2019

My bad! It's a native django-grappelli class name. I will fix it asap. I knew I will screw this up in some way :))

@pasevin
Copy link
Contributor Author

pasevin commented Feb 28, 2019

Should be working now. I checked on the vanilla Django theme.

@dominicrodger
Copy link
Contributor

Awesome - thanks @pasevin. I'll take another look as soon as I can (hopefully by the end of the week).

@dominicrodger dominicrodger merged commit 5262b15 into jazzband:master Feb 28, 2019
@skumarcm
Copy link

skumarcm commented Mar 15, 2020

I am trying to use this package for my recurrence.
I am getting the following error:

recurrence-widget.init.js:7 Uncaught TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'.
at HTMLDocument. (recurrence-widget.init.js:7)
at i (jquery.min.js:2)
at Object.fireWith [as resolveWith] (jquery.min.js:2)
at Function.ready (jquery.min.js:2)
at HTMLDocument.J (jquery.min.js:2)

The node with the id "container" is no where to find.
Where exactly is the above node?

Just a note: I am using version 1.9 as I am still on Python 2.7 and django 1.11

@amirfuhrmann
Copy link

i get the same
recurrence version 1.10.2/django 2.2.2/python3.7.7
any thoughts ?

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

4 participants