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

Fixed PlaceholderField frontend edit (#729) #918

Merged

Conversation

ojii
Copy link
Contributor

@ojii ojii commented Aug 3, 2011

No description provided.

@ojii
Copy link
Contributor Author

ojii commented Aug 3, 2011

Fixes #729 and #896 (was easier to do the #896 fix in this branch, thus the mixing)

@chrisglass
Copy link
Contributor

Ooh niiiice!
Can't wait to review this one properly :)

@chrisglass
Copy link
Contributor

I made a minor change, but unfortunately the feature still fails on a completely vanilla django-CMS.

Here's what I did to test:

  • cd tests
  • drop the sqlite database (if present)
  • syncdb --all
  • migrate --fake
  • runserver
  • go to admin
  • Add a new page + select any template like 2 cols
  • go to frontend
  • switch to edit mode
  • try to add a text plugin to a placeholder.

Fails without javascript errors (it just sits there doing nothing but showing the rotating "loading" image)

@chrisglass
Copy link
Contributor

Ok I found the exact condition for the problem:

Adding FAILS if the "edit bar" is COLLAPSED (looking like a + sign in the top-right corner);
but succeeds when the edit-bar is expanded.

…toolbar is currently hidden.

On CMS.Placeholder.editPlugin, if the toolbar is currently hidden, show it for the edit process (and hide it again if necessary after the edit is done)
Special thanks to @chrisglass for finding this weird edge case
@chrisglass
Copy link
Contributor

Ok, LGTM, I'll merge it in :)

By the way, the "flashing" green highlight when reordering plugins is badass. Thanks @FinalAngel :)

Jonas Obrist added 2 commits August 4, 2011 10:31
 gave his okay for this.

The reason behind this is NOT to take credit from @FinalAngel, but because over time there will be many authors to those files, also the license did not match the one of the CMS (though it was compatible).
chrisglass added a commit that referenced this pull request Aug 4, 2011
@chrisglass chrisglass merged commit cbe6cfd into django-cms:develop Aug 4, 2011
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.

2 participants