Skip to content

Commit

Permalink
@@add-tile form changes
Browse files Browse the repository at this point in the history
- adding 'add_tile' name to form
- after sucessfull save redirect to tile view not to edit view
  • Loading branch information
garbas committed Jun 18, 2012
1 parent 3a04e7b commit 1ff5641
Showing 1 changed file with 3 additions and 15 deletions.
18 changes: 3 additions & 15 deletions plone/app/tiles/browser/add.py
Expand Up @@ -14,7 +14,6 @@
from plone.tiles.interfaces import ITileDataManager

from plone.app.tiles.browser.base import TileForm
from plone.app.tiles.utils import getEditTileURL, appendJSONData
from plone.app.tiles import MessageFactory as _


Expand All @@ -25,6 +24,8 @@ class DefaultAddForm(TileForm, form.Form):
by an ITileType utility.
"""

name = "add_tile"

# Set during traversal
tileType = None
tileId = None
Expand Down Expand Up @@ -81,20 +82,7 @@ def handleAdd(self, action):
type=u'info',
)

# Calculate the edit URL and append some data in a JSON structure,
# to help the UI know what to do.

url = getEditTileURL(tile, self.request)

tileDataJson = {}
tileDataJson['action'] = "save"
tileDataJson['mode'] = "add"
tileDataJson['url'] = tileRelativeURL
tileDataJson['tile_type'] = typeName
tileDataJson['id'] = tile.id

url = appendJSONData(url, 'tiledata', tileDataJson)
self.request.response.redirect(url)
self.request.response.redirect(tileURL)

@button.buttonAndHandler(_(u'Cancel'), name='cancel')
def handleCancel(self, action):
Expand Down

8 comments on commit 1ff5641

@djay
Copy link
Member

@djay djay commented on 1ff5641 Jan 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garbas This commit breaks collective.tinymcetiles. The code below is trying to pickup the json tile data above and insert the tile placeholder into tinymce. What's the reasoning behind changing the behaviour to go to the rendered tile? Doing so doesn't make a lot of sense since the tile is to be inserted not viewed in isolation.

https://github.com/collective/collective.tinymcetiles/blob/master/collective/tinymcetiles/plugin/event.js

@garbas
Copy link
Contributor Author

@garbas garbas commented on 1ff5641 Jan 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can construct above JSON dictionary from info you had before you did this request and rest you can figure out from the response.

i was minimizing the requests you need to add a tile. before this you needed to do 4 requests now its only 2, while still having all the info available.

@garbas
Copy link
Contributor Author

@garbas garbas commented on 1ff5641 Jan 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think what is missing right now is documentation how this works and how someone could use this in javascript, right?

@djay
Copy link
Member

@djay djay commented on 1ff5641 Jan 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@add-tile view doesn't have a url until the code above is called. After this redirect the view has no JS and is completely controlled by the tile. As I said, it doesn't make any sense to redirect to the tile rendering itself since it's meant to be embeded not viewed in isolation in the same context in a popup dialog. Redirecting back to the @@add-tile view makes sense since it includes JS which will let you close the dialog.

@djay
Copy link
Member

@djay djay commented on 1ff5641 Jan 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best I can think of is trying to add a listener on the iframe in the popup. But this isn't possible to do nicely.

http://stackoverflow.com/questions/2433966/iframe-contents-change-event

I think we should change it back to how it worked before.

@garbas
Copy link
Contributor Author

@garbas garbas commented on 1ff5641 Jan 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think i'm following you. i'm busy next 3-4 days but i would to continue on this discussion with you after that.

@djay
Copy link
Member

@djay djay commented on 1ff5641 Jan 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a working version the london sprint in a few days times time. It works with this code back in so I'm going to go ahead and put this code back in. If you want to optimise it later then please do so in a way that keeps it working. see #4

@garbas
Copy link
Contributor Author

@garbas garbas commented on 1ff5641 Jan 27, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.