CRM-9798 In place editing & client templating

lobo: when adding the new jquery module in a separate file, I added it in
templates/CRM/common/jquery.tpl
templates/CRM/common/jquery.files.tpl

Is this enough or is there something else to do?

Committed revision 39344

Issues reported by dlobo:
1) when updating with the wrench (the existing admin form), the new layout isn't properly updated
2) when pressing cancel in the admin form (after the wrench), the message is "saving..."

The root cause is that I'm using an existing form with the ajaxForm plugin (so you can embed the form into an existing page), but that the form isn't aware that it's an ajax form, so it always return the "normal next step" (eg the list of fields in the profile), that the page that embed the form sees as a big html blob

Two solutions I can see

1) keep hacking around the "the form doesn't know it's embeded"
so for instance I add a onclick to the cancel and handle it separately
and that if the user submit the form, I generate a new call to get the field (ajax api) to know what is the saved result

2) change civi form so it's "ajax aware"

eg. if called with &snippet=6 (or context=ajax), instead of returning the usual next step (the html that contains the list of fields) it returns a json containing the fields

Two hooks hook_civicrm_post and hook_civicrm_postProcess contains already a list of data that would be much more useful than the html blob (in json form)

@dlobo @ kurund

Would it be possible to change the core to implement snippet=6 or context=ajax or anything that would allow than CRM_Form returns a json instead of an html on success (html form on error is fine as it is now)?

Otherwise, will go plan A if you don't have a better suggestion

X+

Kurund has added a mode to the form called:

"json=1" (we can change this)

which renders the form in json rather than html.

We can potentially use and extend that

after checking with kurund, the json=1 is not what needed. What I wanted was to get the result as a json, not the form.

lobo suggested a solution: using civicrmDestination as a hidden param of the form, with /civicrm/ajax/rest?entity=xx&action=get&id... as the result

This works in the crmForm scenario: you edit an entity by ajax loading ?snippet=1 the form in a iframe/ui-dialog, ajaxForm it so on submit you aren't re-loading the frame but sending it ajax with the redirect, that redirect to the ajax get of the entity, so you get a json of the data after modification

Could be optimised further by avoiding the redirect & directly returning the data, but good enough for now.

@dgg,

So for our meeting this morning.
Applied to profile fields, custom fields & options of custom fields

/civicrm/admin/custom/group/field?reset=1&action=browse&gid=1
civicrm/admin/custom/group/field/option?reset=1&action=browse&gid=1&fid=2

ttyl

X+

from @dgg

0. test the is_required and don't accept an empty value if it's the case

1. need to add a simple filter against xss strings - i can put something like this in the label: <script>alert('hi')</script> - but shouldn't be able to

2. Something is truncating longer strings at 46 characters.
I entered a much longer value - and the ajax url was correct, but the returned json had truncated the value to 46 characters. Since the Label field can hold 255 - not sure where truncation is occuring

3 (xd) We can actually check the lenght from getfields and truncate if needed

4. (dgg) would also be great if setValue could validate against data type (string, integer initially - then date etc.). Not sure how UI should deal w validation error messages (or it there's already a mechanism in place). Perhaps an alert() box might work. Also need to make sure there's a way for user to "cancel" the update if they're trying to enter an invalid value and decide to quite.

It does already test (boolean int string).

I will add the xss on string+length

for the error message, I pushed it to 4.3: http://issues.civicrm.org/jira/browse/CRM-9856 and I think that's important for the UX all pages do the same

In the meantime, altert is good. Should I go for something generic ("Error, couldn't save the value") or display the actual error message, that might be fairly technical?

Generic seems ok but defining standard messages relevant to the validation error would be better:

If required: "Please enter a value for $fieldName."
If type INT: "Please enter a whole number for $fieldName."
If type float: "Please enter a numeric value for $fieldName"

I've implemented on-hover dash border style (replaces purple background) - which is consistent w/ UI "cues" on the Contact Summary inline editing. Also implemented edit inline for Profile titles (UFGroup) and cleaned up implementation for Custom fields and custom field option values (to use span instead of td).

Assigning back to you to check this issue (plus the validation work):

• Implemented editable for Custom Group titles - but it's not working. Might be some error I'm not seeing in CRM/Custom/Page/Group.tpl or ?? I get the following error when I press enter to save (using Firebug):
  e is null
  http://civicrm42/sites/civicrm42/modules/civicrm/js/jquery/jquery.crmeditable.js?m0j5iw
  Line 190

One more "bug": crm-editable seems to break the Disabled style display for inline Disable / Enable function. To recreate:

• go to Profile Fields listing
• click Disable action and click OK
• the is_active property for the field is actually updated (unchecked) BUT the disabled class style isn't applied. Prior to adding the crm-editable stuff, you would immediately see the row in red / strike-through. Now you need to reload the page to see that style.

The error on the custom field set was the format of the id CustomGroup_

.

Changing so the message is clearer

The html syntax is weird. Might be good to add a comment to ack that is was a quick and dirty hack to avoid more WTF?

(using tr as td by setting width:49% and display:inline-block)

I think it's time to rewrite the action tool (the php generating html that contains "onclick do something" javascript), at least the code to handle the enable/disable that would benefit from some love.
(4.x)

Anyway, the problem was the code assumed an id of row_xxx and that it now Entity-xxx, and that it is hardcoded into the onclick javascript that is on the html blob that is generated from the php (see below

I've modified the javascript code so

• if it doesn't find any {row}

  _id, it looks for

  {entity}

  -id

• if it still doesn't find anything, it reloads the page (better a refresh than silently failing)

and I've modified for UFGroups so the prefix for the row id is "UFGroup"

Obviously, the php->html blob should know nothing about the id and it shouldn't be harcoded, but the click function should use $(this)->closest("tr").attr(id) to fetch the id from the current row

X+

Ok, after having banged my head for a while on the "bug", I did test on 4.1 and that was already buggy. Closing this issue and opening a new one to fix