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

WIP elfinder volume driver #28

Closed
sjopet opened this issue Jul 25, 2013 · 9 comments
Closed

WIP elfinder volume driver #28

sjopet opened this issue Jul 25, 2013 · 9 comments
Milestone

Comments

@sjopet
Copy link
Member

sjopet commented Jul 25, 2013

I added a new branch 'elfinder' with the work on the volume driver I did so far.
Most basic functions like creating folders, uploading files, removing, moving, copy, download etc work. But because there is a lot to do still I figured it would be easier to add a branch then to do a pr.

Among the things that don't yet work are previewing, cropping/resizing and opening files in the browser. But I felt I got to a point people could try it out, review code and discuss design issues.

to run the code you have to:

fm_elfinder:
    locale: %locale%
    editor: simple
    connector:
        roots:      
            media:
                driver: cmf_media.adapter.elfinder.phpcr_driver
                path: /cms/media
                upload_allow: ['all']
                upload_max_size: 2M
  • navigate to http://you_sandbox_domain/app_dev.php/elfinder
@rmsint
Copy link
Contributor

rmsint commented Jul 26, 2013

Will have a look. We also need to check it later with #27 for Create.

@sjopet
Copy link
Member Author

sjopet commented Jul 26, 2013

@rmsint Thanks I hope I didn't reinvent the wheel to many times here, kind of losing oversight at this point. Will try and get back on track with this after the weekend.

@rmsint
Copy link
Contributor

rmsint commented Jul 30, 2013

Have looked at it but not very thorough due to illness, some remarks/ ideas/ questions so far:

  • code style - elfinder does not follow the same standards as we do - probably needs to be checked
  • _dirname etc . - maybe we want to use the phpcr-utils here or the in CreateBundle #27 created MediaManager
  • sync with MediaManager probably
  • we indeed need to implement the cmf:file and cmf:image mixins to not get the node-type error
  • something is going wrong with the path and url of a file, fe. double click on a file goes to http://local.cmf//cms/media/test.png
  • do we need to check some requests for performance? - like the search
  • maybe for the preview we can use the display url added in CreateBundle #27
  • any idea what is needed for croping/resizing? maybe disable it for now?
  • media root creation - currently handled by an initializer but that creates a Generic document - with some adaptions it then works

Also did some tests with PHPCRDriver::_save and updated it a little bit. I pushed the updates to the branch elfinder.

@rmsint
Copy link
Contributor

rmsint commented Aug 26, 2013

Rebased on latest master, added configuration, cleaned up the code a bit and checked and fixed some features. Also disabled: extract, archive, rename and resize. Renaming can be enabled again once we find and fix the error it causes.

Also reverted the size and contentType of the File to be persisted. Otherwise ``PHPCRDriver::_mkfile` returned an error. If we investigate and find the error the change can be added again.

@dbu
Copy link
Member

dbu commented Aug 26, 2013

if this is now basically working, even without all optional features, i think we should merge it into master. wdyt?

regarding cropping and resizing, that sounds like going out of scope to me, we already have imagine to serve images with a specific filter. this would be better than modifying the original image. if we can't make elfinder use that, we could use imagine to implement this again. but if we can find a way to have the user specify which imagine filter he wants, that would be a more elegant.

@rmsint rmsint mentioned this issue Aug 26, 2013
6 tasks
@rmsint
Copy link
Contributor

rmsint commented Aug 26, 2013

Yes sounds like a good plan, created PR #46 so we can comment on the code.

@dbu
Copy link
Member

dbu commented Aug 26, 2013

can you move whatever is still relevant here to #46 or/and into a PR against the symfony-cmf-docs and close this issue please?

@rmsint
Copy link
Contributor

rmsint commented Aug 26, 2013

yes, will move the installation to the media documentation

@rmsint
Copy link
Contributor

rmsint commented Aug 26, 2013

See symfony-cmf/symfony-cmf-docs#251 and #46

@rmsint rmsint closed this as completed Aug 26, 2013
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

No branches or pull requests

3 participants