[Product-Developers] Questions about collective.cover

Franco Pellegrini frapell at gmail.com
Fri Sep 21 15:07:13 UTC 2012


Hi

On Thu, Sep 20, 2012 at 6:10 PM, Martin Aspeli <optilude+lists at gmail.com>wrote:

> Hi,
>
> I'm just digging into collective.cover a bit more, and I have a couple
> of questions about the implementation that would help me understand it
> better. Most of this is just about trying to understand whether we
> should be pushing some of the tile related stuff upstream to
> plone.tiles or not.
>
>  - What are screenlets for?
>
>  - Why do we need IPersistentCoverTile?
>

IPersistentCoverTile is the interface that base.PersistentCoverTile
implements. This class inherits from PersistentTile and implement a couple
of things on top that we needed in cover.


>  - What is the idea behind populate_with_object() in that interface?
> The idea of a tile being populated with a content object is confusing
> to me. Is it something every tile needs?
>

We had a requirement where they wanted users to be able to get a list of
objects from the site, drag them and drop them inside a tile, and then the
data from the object would be copied to the tile. This seems to be very
useful for editors in news sites, where they get a list of news and they
drag and drop them inside the tiles where they want it to be shown.
Not every tile need this of course, but most of the tiles implemented in
collective.cover do, so i thought i leave that method there as a helper to
check permission before populating the tile.

  - Why do we have so many get_* methods in IPersistentCoverTile? Why
> not just access through the tile data API?
>

"get_tile_configuration" is a helper method that will get the configuration
for a tile. Remember from my other mail that i told you i implemented a
configure view similar to the edit one, where you could save specific
configurations for fields ? well, using this you get that config.

"get_configured_fields" will iterate over all fields and get a dict with
the field data, config, etc useful for the render view

  - Suggestion: Instead of storing allowed/disallowed groups to edit a
> tile on the tile itself, maybe delegate this to an adapter? It would
> allow c.cover to work with tiles not derived from its special
> IPersistentCoverTile
>

I thought of doing this the same way as the data and configuration, that
way, if we remove the tile, then we remove all this info too... if we do it
with an adapter, then we would need to write additional code so we do not
end up with inconsistent info.


>
> Side note: I have a hope that most tiles will not need to be
> persistent. There is something very clean and lightweight about only
> storing configuration parameters in the query string in a tile href,
> and annotations are somewhat opaque.
>

I agree, but this data (in some situations) need to be saved somewhere,
annotation, utility, etc...


>
>  - What is get_tile_configuration() for? Why are we looking up a
> screen from the tile? Should the dependency not be the other way
> around?
>  - What is the purpose of ITilesConfigurationScreen? It seems to just
> store things in annotations. Why the extra indirection?
>

This was an infortunate naming for that... basically is just an adapter
that will store and retrieve fields configurations for each tile.


>
>  - What is get_configured_fields() for?
>
>  - What is the thinking behind IBasicTileData? What is the use case
> for most/all tiles having title, description, image, date (which
> date?) and subjects (which subjects?) fields?
>

I think you are mixing "basic" with "base". Maybe we should think of a
better name to avoind this confusion, IBasicTileData is a basic tile which
allows you to create a tile with that information (basic information)


>
>  - Why does IBasicTileData have get_* methods? Why not just access
> through the tile data API?
>

I don't like to access attributes directly, but to have methods to do it.
This way if in the future field names change, or maybe we need to
preprocess or whatever the info before returning it (like for instance
RichText), you only need to do it in one place. I know this is not very
"pythonic", old habits...

  - What is the thinking behind the ConfigureTile namespace? How does
> it differ from the stuff in plone.app.tiles?
>

This is where the @@configure-tile based on the @@edit-tile (as i explained
in my previous email :) )
plone.app.tiles does not provide such a view, unless i'm missing soething...


>  - Same about the edit form in edit.py - what is missing from p.a.tiles?
>

Since p.a.tiles does not implement permissions, and we did, then I had to
override this...


>
> Anyway, sorry for the long list of questions. It does look really
> clean and well written, just trying to understand the rationale. I'm
> keen that we avoid a situation where people build a bunch of tiles for
> collective.cover that ends up being incompatible with other uses of
> tiles, or vice-a-versa. So, perhaps, we should push the relevant bits
> of this up into p.tiles or p.a.tiles. It may also be worthwhile to
> then release the tiles independently so that they can be used outside
> of c.cover.
>
> Martin
> _______________________________________________
> Product-Developers mailing list
> Product-Developers at lists.plone.org
> https://lists.plone.org/mailman/listinfo/plone-product-developers
>

A lot of the functionality implemented in collective.cover maybe didn't
have *all* the thought needed to be 100% general, we only had a couple of
days, and some specific use cases, so, I would guess that if some
functionality seems to be useful, then some work may be needed before
getting it in p.a.tiles

Kind regards,
Franco
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.plone.org/pipermail/plone-product-developers/attachments/20120921/750ff017/attachment.html>


More information about the Product-Developers mailing list