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

list tiles can no longer link to collections #713

Closed
fredvd opened this issue May 17, 2017 · 15 comments · Fixed by #714
Closed

list tiles can no longer link to collections #713

fredvd opened this issue May 17, 2017 · 15 comments · Fixed by #714
Assignees
Labels

Comments

@fredvd
Copy link
Member

fredvd commented May 17, 2017

Plone 4.3.14, plone.app.contenttypes 1.1.2

With the drag and drop feature between tiles (#487) another feature not described in the issue was added: if you drop a collection on a list tile, all the items in the collection will be added as individual items. I found out about this from the end-user.rst documentation where the feature is described:

If you drop a folder or a collection into this tile, it will take every item into the folder or into the collection and insert it until it reaches the max items specified into configuration.

However it does not work as described when I drop a folder on a list tile, at least not with plone.app.contenttypes (default dexterity CT's) installed. Then the folder itself is added. (1)

Very fancy, but how do I link to a collection itself in a a list tile? This is no longer possible but was possible before, adding to the confusion that I do see links to collections in existing list tiles in our sites, but these were probably added before this was released.

(1) The only work around I've found is to put the collection you want to link to in a folder, set the collection as the default view and drop the folder on the list tile in the compose screen. Lucky me that the end-user description for dropping folders on list tiles is not correct.

@fredvd
Copy link
Member Author

fredvd commented May 17, 2017

Making folders as aliases for folders cannot be a permanent solution though. pressing SHIFT while dropping a collection on a tile to make it a link like it used to do could be a fix, but modifier keys are a PITA for user experience, it would need to be documented as a text in the contentchooser as well.

Any other idea's? My first hunch is to patch this out of c.cover for the moment for our customers, where should I do that?

@hvelarde
Copy link
Member

may be we can add a dialog asking the user what to do: adding the collection or adding the items.

@agnogueira comments?

@hvelarde
Copy link
Member

hvelarde commented May 17, 2017

@fredvd we were discussing here and that issue was probably created in this PR #591; we think we can do the following:

  • dropping a collection into a list tile will result in the collection being listed in the list
  • dropping a collection into a carousel tile will result in the first items in the collection populating the carousel if they have the required image

we'll need to update the documentation also.

do you agree?

@fredvd
Copy link
Member Author

fredvd commented May 18, 2017

@hvelarde I Agree: unpack a collection to its items for the carousel, but don't do this for the list tile.

On the carousel tile it might make sense (I assume there was a use case and user request for this) and you wouldn't want to publish a collection object in a carousel, but for a generic list tile it's the other way around. Furthermore if you want a collection in a list tile the preferred option to keep it dynamic would be the collection tile instead. list tiles are for manual picking and managing content.

Adding a dialogue/popup would be nice and keep the feature, but the use case isn't really there for list tiles, adds a lot of editor hassle for managing list tiles (every item dragged pops up the selection window) and technically a lot of robot tests would have to be updated and be more error prone for this, when I think turning this off for the list tile will suffice.

The only non intrusive way I can think of is how windows/mac manages copy/move/shortcut on the desktop: add a symbol to the mouse pointer for the action, use modifier keys to change the action. A plus icon by default, and pick a modifier key to show a symbol that communicaties unpack. But it's again a lot of work for use case I think is not there for the intention of the list tile.

@fredvd
Copy link
Member Author

fredvd commented May 18, 2017

@hvelarde The feature might have been fixed and enabled by #591, but the documentation for how it should work was added before that in #573 .

The documentation has to be fixed, but I still wonder about the same feature described for folders in the documentation change (which fortunately doesn't work). Was this really a use case to also by default unpack folder contents instead of the folder in a list tile?

@rodfersou
Copy link
Member

rodfersou commented May 20, 2017

@fredvd @hvelarde I did a simple mockup on how this modifier key changes could be implemented here

Also I think a modifier would be a nice option at the move tile feature to make a copy of the entire tile instead of move.

@rodfersou
Copy link
Member

need to click at the result page first to start working (thanks to codepen editor)

@rodfersou
Copy link
Member

@fredvd I can see looking at the code that list tiles always behave this way when you drop a collection, so this is a new feature.

@rodfersou
Copy link
Member

@smcmahon
Copy link
Member

If we follow this strategy, then anyone who updates collective.cover, but not PFG, will be unable to restart Plone clients. Anyone who updates PFG, but not c.c, will be unable to maintain their tiled forms.

Wouldn't it be better to retain the interfaces and make the adapters do nothing? I realize it's not clean, but why increase update fragility if we don't have to?

@hvelarde
Copy link
Member

hvelarde commented May 24, 2017

@smcmahon I agree; we can leave the empty adapters with a deprecation warning asking to upgrade to a newer version of PFG on next version; after that I'll remove this completely from collective.cover in the next+1 version.

I just want to understand: are you really using those adapters in PFG? can you explain your use case?

@smcmahon
Copy link
Member

The only use is to make PFG forms visible to collective.cover. PFG does not itself use the adapter:

If the interface existed but the factory did nothing, nothing would break in PFG.

@fredvd
Copy link
Member Author

fredvd commented May 29, 2017

@fredvd I can see looking at the code that list tiles always behave this way when you drop a collection, so this is a new feature.

@rodfersou Are you sure? Because I can send you links to covers that were edited in december 2014 where collections have been added to list tiles just fine and the page hasn't updated since. But new covers cannot be recreated like that. That's how we found out, with a end user trying to recreate an existing cover.

Only caveat: we're using plone.app.contenttypes dexterity default types.

@hvelarde
Copy link
Member

hvelarde commented May 29, 2017

@fredvd is right; I never understood what those adapters were for until now, also seems IDGB project defines it's own list tile because of this limitation.

another example of why is a bad idea to fork code instead of fixing behaviors upstream.

CC @idgserpro

@idgserpro
Copy link
Member

idgserpro commented May 29, 2017

I never understood what those adapters were for until now, also seems IDGB project defines it's own list tile because of this limitation.

IDGB project defines it's "own list" because it couldn't wait for collective.cover release process: if you check the historic record, you can see they're similar, but collective.cover continued evolving while brasil.gov.tiles stagnated.

We're, right now, removing the list tile from brasil.gov.tiles and use collective.cover's list to fix plonegovbr/brasil.gov.tiles#145, exactly because we think forks are a really bad idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants