Hi all,
I have a need for text fields, text areas and multiple file uploads, so I have put together some changes for the maintainers to consider and for people to test. I've personally manually tested a bunch of scenarios and I'm ready to proceed with my project.
This is in part a follow on to "Textarea support for attribute display types" https://drupal.org/node/496450 I decided to open up a new thread to minimize confusion.
I have used ubercart 7.x-3.x-dev from Oct 24, 2013 and uc_attribute_files 7.x-1.x-dev from Oct 19, 2013.
I cleaned up the code regarding $attribute->display and defaults. I have kept the option default settings to be applied only if required is set; as it has been.
I don't know the history of these modules, so I apologize in advance if I missed something, or stepped on any toes. I'm not set up to generate patches, so I've attaching the affected files.
Changed items or areas are labeled with my initials BM to make them easier to find.
I hope this helps the common good.
-Bob
Comment | File | Size | Author |
---|---|---|---|
#2 | uc_attribute_files.module-2135531.patch | 8.81 KB | bmarcotte |
#2 | uc_attribute_files.inc-2135531.patch | 394 bytes | bmarcotte |
#2 | uc_attribute.module-2135531.patch | 4.15 KB | bmarcotte |
#2 | uc_attribute.admin_.inc-2135531.patch | 15.78 KB | bmarcotte |
BM_Updates.zip | 25.31 KB | bmarcotte |
Comments
Comment #1
longwaveThank you for your contribution. It would help immensely if you could post your changes as patch files, as that makes your changes much easier to review. See https://drupal.org/node/707484 for instructions on how to create a patch.
Comment #2
bmarcotte CreditAttribution: bmarcotte commentedI've uploaded an attempt at patches. no guarantees expressed or implied :)
Comment #3
TR CreditAttribution: TR commentedBecause the issue at #496450: Textarea support for attribute display types contains all the context and feedback for the textarea feature request, I think this current issue should be closed and continued over there. Your patches contain some of the same problems that the original had, which are explained in that other thread. I personally am against committing a patch that only adds partial support for textarea attributes. If these are to be added, then it's only a little extra work to make them work everywhere just like all the other attribute types. And if they can't be made to work everywhere, then we have a problem - I'd like to know that before any patch is committed rather than after.
Also, you're mixing two separate and unrelated new features here. One of which is evidently to bring a contributed module into core Ubercart? These two things should be split into separate issues so they can be worked on, evaluated, and committed independently.
Comment #4
bmarcotte CreditAttribution: bmarcotte commentedI'm not totally disagreeing, but I see these points:
This is what I have to contribute, You are free to use it, parse it out, or toss it. I'm not saying the job is done, but without funding, I cannot continue to jump through hoop after hoop to make making everything work.
-Bob