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

Use file manager for uploading images #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jk3us
Copy link

@jk3us jk3us commented Dec 28, 2012

This change uses Moodle's nice file_storage mechanism for management uploaded images.

In the upload_image_form.php form, instead of a dropdown for type and a single file picker, that form now has a file_manager for each image type (border, seal, etc), you can drag multiple files into each and delete old files. (it lets you delete files that are in use, be careful!)

In the mod_form, it lists files that are in the codebase and in this the proper file area (but no longer the hardcoded data_dir directory *)

When generating the pdf, files in the file area get pulled and inserted appropriately.

  • Warning: What this doesn't do (yet) is migrate in any images in the hardcoded data_dir from previous versions. That will need to be implemented in the module's upgrade script so no one loses their images.

I can hopefully add the upgrade migration part and test some more next week, but wanted to send this along since I won't be looking at this again until after new year. Feel free to test and provide feedback, or go ahead and do my unfinished work for me :)

@jk3us
Copy link
Author

jk3us commented Jan 2, 2013

Question: Do you prefer pull requests or issues in the moodle tracker for changes like this?

@jk3us
Copy link
Author

jk3us commented Jan 3, 2013

Tracker Issue: https://tracker.moodle.org/browse/CONTRIB-4087

@mdjnelson
Copy link
Owner

Hi jk3us, tracker is nice as I may forget about the github issue. :)

I am going to have to test this on my local machine and look at the code before I merge it into the this repository. Sorry for the delay. I will be dedicating some more of my time shortly into tracker issues for the certificate.

@jk3us
Copy link
Author

jk3us commented Jan 4, 2013

That's fine, the more testing the better. Let me know if there's something that could be done better.

I'd also like to work on the option uploading images in the mod_form itself if you have a very specific certificate. Also, svg images, since I think the version of tcpdf that ships with Moodle 2.4 handles them better than before.

@jk3us
Copy link
Author

jk3us commented Jul 9, 2013

Hey, has anything been done with this change? I just had to upload some new borders and was reminded that this change would be nice to have :)

@mdjnelson
Copy link
Owner

Hi Jay, any free time I have to develop I spend on the new certificate module I am creating (https://github.com/markn86/moodle-mod_customcert) which has this functionality but still is under development and not ready yet. If you are happy to rebase your changes I promise to look at this asap and provide feedback.

@mdjnelson
Copy link
Owner

TBH, this got lost in the backlog of issues to do for the certificate module which I have been neglecting to take on due to any free time spent on the new module.

@jk3us
Copy link
Author

jk3us commented Jul 10, 2013

I will try to rebase soon, but I will also take a look at the new cert module. Do you have any description or screenshots of it and how it's different? Any ideas of when it will be ready to release?

@mdjnelson
Copy link
Owner

Hi Jay, I actually gave a talk about it at the moodlemoot. Please see http://www.youtube.com/watch?v=NNnoIUUC2pA#t=18m30s and https://moodle.org/mod/forum/discuss.php?d=229683 for reference.

@francoisbruneau
Copy link

Hi Jay, Hi Mark. Any updates on this PR? Can anything be done to help? Thanks!

@jk3us
Copy link
Author

jk3us commented Aug 4, 2016

I have a new job where I'm not working with moodle anymore, so I don't think I'll be able to do any more with this.

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

Successfully merging this pull request may close these issues.

4 participants