plone / plone.app.tiles

Plone UI integration for plone.tiles
http://pypi.python.org/pypi/plone.app.tiles
Other
6 stars 7 forks source link

reverse commit 1ff564180 to make popups for inserting tiles work. e.g. i... #4

Closed djay closed 10 years ago

djay commented 10 years ago

see 1ff564180

jensens commented 10 years ago

What is the state here? Looks like it causes a bunch of test failures.

datakurre commented 10 years ago

@simahawk As you worked for tiles' forms at Mosaic Sprint, would you be able to look through this pull?

simahawk commented 10 years ago

What I can say by just looking at the code is that I'm sure is working fine without this change, both for p.a.mosaic and two other projects I'm using tiles with.

Also, I'm fine with having the add form to redirect to the view. This allows you to get the rendered content via ajax response and inject it on the fly where you need it.

Moreover, is a view, so you can override it in some case (maybe we can simplify this by adding a "nextURL" method) based on context or marker interface on the request, or play differently via request var (like format=json) or whatever.

datakurre commented 10 years ago

@djay Would you be okay with the following compromise: we'd redirect directly to rendered tileview (see @simahawk's comment), but we would also add tiledata into querystring. (Also, we need to be careful where, becase _tiledata (the same with underscore) would override tile data manager content.)

An alternative would be to add tile data as response headers, but that would require changes to tinymcetiles.

datakurre commented 10 years ago

Ahem. I messed this up with my merging changes from master, so I open a a new pull request soon.

coveralls commented 10 years ago

Coverage Status

Changes Unknown when pulling e8dfd9088bd38234f0dcac458cb59a87fce5c9a7 on makepopupwork into \ on master**.

datakurre commented 10 years ago

That didn't quite work out. Well, anyway.

@djay I merged master changes to makepopupwork-branch and refactored it to both return to tile render view and contain tiledata-querystring. See earlier comment about why the render view is useful

https://github.com/plone/plone.app.tiles/pull/4#issuecomment-49001670

I'm not sure why this pull shows so much changes, but the final diff to master can be viewed at:

https://github.com/plone/plone.app.tiles/compare/makepopupwork?expand=1

If @djay could confirmt taht this still works with tinymcetiles, we could merge this to master.

coveralls commented 10 years ago

Coverage Status

Changes Unknown when pulling e8dfd9088bd38234f0dcac458cb59a87fce5c9a7 on makepopupwork into \ on master**.

coveralls commented 10 years ago

Coverage Status

Changes Unknown when pulling e8dfd9088bd38234f0dcac458cb59a87fce5c9a7 on makepopupwork into \ on master**.

djay commented 10 years ago

sorry @datakurre I missed this whole discussion. All I wanted was a way to integrate tile creation dialog and this didn't seem to be provided by the change rok had made. If there is a clear way to do it now thats fine.

datakurre commented 10 years ago

@djay We need to be able to get both rendered tile (most of the time) and edit form (for more complex tiles) in Mosaic when adding a new tile. And somehow to know, which one is the correct action for each tile. Probably this pull is not yet enough, so @simahawk tries to look into this issue after the holidays (he needs this to be able to add individual portlet as tiles [everything except this is already working for that]).

datakurre commented 10 years ago

@simahawk After all, it may be easier to default on edit_view as add tile view redirect, but add X-Tile-Url header for the edit view (it's already added for normal tile views) and then configure Mosaic to skip the edit unless configured otherwise.

datakurre commented 10 years ago

Oh, I revert the previous comment. The current behavior is, what we want and works with @simahawk's multi-step tiles. So

  1. User selects to add a tile
  2. Tile add form with validation is shown
  3. Add form may require some fields to be filled (on invalid input, we see a validation error)
  4. After successful submit, we see the rendered tile

(If the tile addition is wizard like, we continue to see forms, until there is X-Tile-Url header from plone.tiles to mark that we got the final tile response.)

In my opinion, it would be weird to redirect to the edit form after a successful add.

@djay Let fix this so that c.tinymcetiles can simply register its own @@add-tinymcetile by simply overriding the nextUrl method as @simahawk proposed.

djay commented 10 years ago

@datakure I'm fine with that, as long as there is a method that works. However one reason I was holding off on tinymcetiles is that I was waiting to see if mosaic is a solution that would replace pages so we can have folderless plone.

On Thu, Aug 7, 2014 at 12:17 PM, Asko Soukka notifications@github.com wrote:

Oh, I revert the previous comment. The current behavior is, what we want and works with @simahawk https://github.com/simahawk's multi-step tiles. So

  1. User selects to add a tile
  2. Tile add form with validation is shown
  3. Add form may require some fields to be filled (on invalid input, we see a validation error)
  4. After successful submit, we see the rendered tile

(If the tile addition is wizard like, we continue to see forms, until there is X-Tile-Url header from plone.tiles to mark that we got the final tile response.)

In my opinion, it would be weird to redirect to the edit form after a successful add.

@djay https://github.com/djay Let fix this so that c.tinymcetiles can simply register its own @@add-tinymcetile by simply overriding the nextUrl method as @simahawk https://github.com/simahawk proposed.

— Reply to this email directly or view it on GitHub https://github.com/plone/plone.app.tiles/pull/4#issuecomment-51432117.

datakurre commented 10 years ago

@djay I added nextURL-methods and I fixed tinymcetiles' Travis to pass with the current p.a.tiles.

djay commented 10 years ago

@datakurre nice. thanks