Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Pull request: django-wysiwyg #11

Merged
merged 1 commit into from
Dec 12, 2011
Merged

Pull request: django-wysiwyg #11

merged 1 commit into from
Dec 12, 2011

Conversation

mhulse
Copy link
Contributor

@mhulse mhulse commented Jun 24, 2011

Added change_form.html to repo; Added a "extrastyle" block with (optional) CSS fixes for admin.

Your docs should now match the structure of repo/package (in other words, templates/admin/change_form.html now exists).

Within the change_form.html file, I added a new block with info about the styles... I did not think there was a need to modify the README.rst file... I figure most people will crack open the change_form file and read the comments in there.

Hopefully this meets your standards... Let me know if you want me to change something.

Btw, I really love your robust documentation! I have been using Markdown for my docs, but I think I may switch to ReStructuredText for my next project. :)

I hope that helps! Thanks again for sharing this code with the rest of us!!!

Cheers,
Micky

@peppelorum
Copy link

Works for me!

@hsparikh
Copy link

This change hasn't been merged onto master trunk, right? Would be great if its done, as it will save time for others who want to use wysiwyg for admin purposes.

@pydanny
Copy link
Owner

pydanny commented Dec 12, 2011

Done! Sorry for the delay!

@pydanny pydanny closed this Dec 12, 2011
@pydanny pydanny reopened this Dec 12, 2011
pydanny added a commit that referenced this pull request Dec 12, 2011
Pull request: django-wysiwyg
@pydanny pydanny merged commit e65b3ec into pydanny:master Dec 12, 2011
@vdboor
Copy link
Collaborator

vdboor commented Dec 12, 2011

This diff is a bit scary, doesn't this cause template loading loops in Django 1.3?
There is a admin/change_form.html template that includes admin/change_form.html.

@pydanny
Copy link
Owner

pydanny commented Dec 13, 2011

I tested this morning. But will double test just to be sure.

@pydanny
Copy link
Owner

pydanny commented Dec 13, 2011

@vdboor Tested this again, this time more carefully. This does cause an infinite loop. Making changes now.

@mhulse
Copy link
Contributor Author

mhulse commented Dec 13, 2011

Sorry about that. I am not sure why I never noticed. :(

@pydanny
Copy link
Owner

pydanny commented Dec 13, 2011

@mhulse No worries - these things happen. The big mistake was me not following my own docs during testings. ;)

@vdboor and @mhulse I just pushed up a correction to docs and templates. Can you guys take a look and confirm it's good?

@pydanny
Copy link
Owner

pydanny commented Dec 13, 2011

Just pushed up 0.4.1. I'll do a 0.4.2 if need be you guys find any issues with it

@mhulse
Copy link
Contributor Author

mhulse commented Dec 13, 2011

Thanks Pydanny! Again, I am really sorry if I caused you (or anyone else) any inconvenience. I am looking forward to seeing what diffs you made so I can learn from my mistake(s). Have a great day and thanks again!

@pydanny
Copy link
Owner

pydanny commented Dec 13, 2011

@mhulse Putting that change_form at the level of base admin/change_form.html means we get an infinite loop because it tries to extend itself. See how I stuck it in a subdirectory that encourages renaming?

In my own tests I just moved it to the app level and was happy, I didn't follow the necessary pattern. And the docs didn't reflect how things work and that sort of thing will cause more grief then a bug or error.

In actuality, this sort of thing is harder to test then algorithms in code because you are instructing the user to read docs and move files around - not writing tests that are static and hence deterministic. It's why even the best of us will ask for help in reviewing documentation and writings. It's why good authors trying to educate others (Zed Shaw, Wesley Chun and Reinout van Rees) asks publicly for proofing of his educational writings.

@mhulse
Copy link
Contributor Author

mhulse commented Dec 13, 2011

@pydanny Ahhh, I understand now. Thank you for the detailed explanation, I really appreciate it. :)

I will be sure to follow more strict testing guidelines for any future pull request.

Thanks so much for all of your pro help, advice, and this useful bit of code.

Have an awesome day!

Cheers,
Micky

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants