Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ajax image upload #1903

Merged
merged 4 commits into from Aug 27, 2012

Conversation

Projects
None yet
4 participants
Owner

ugisozols commented Aug 27, 2012

Originally - #1739

Owner

ugisozols commented Aug 27, 2012

This probably needs to be squashed?

Owner

parndt commented Aug 27, 2012

Squash-ed

This pull request fails (merged 77d5016 into 3223ad9).

Owner

parndt commented Aug 27, 2012

@ugisozols looks like there's a failing spec ;-)

This pull request passes (merged 40b68ae into 3223ad9).

Owner

parndt commented on 40b68ae Aug 27, 2012

👍

parndt added a commit that referenced this pull request Aug 27, 2012

@parndt parndt merged commit 8fbf97c into master Aug 27, 2012

1 check passed

default The Travis build passed
Details
Owner

ugisozols commented Aug 29, 2012

I did a poor job while reviewing #1739. It adds several things which make some code useless:

  • every image is sent in a separate post (instead of pushing an array of images) to create action so there's no need to check for array stuff
  • validations are happening on the client side so the validation code in model isn't used anymore (valid? checks in controller are redundant too)

Don't get me wrong @jipiboily - I really like the feature but I'm thinking about reverting this and doing it differently.

Contributor

jipiboily commented Aug 29, 2012

No problem @ugisozols. This was an extension at first and implemented it almost as is instead of doing it from scratch, which might be a better idea.

I have minimal experience with Refinery (only one small project, well, project was large, but Refinery's use was tiny) and thought that changing as little as possible would be best to make sure I wouldn't break a non-JS fallback somewhere.

Feel free to make it better! :)

ugisozols added a commit that referenced this pull request Sep 5, 2012

Revert "Merge pull request #1903 from resolve/imajax"
This reverts commit 8fbf97c, reversing
changes made to 3223ad9.

Conflicts:
	changelog.md
	images/app/controllers/refinery/admin/images_controller.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment