Let's use a bit of JQuery to only show the CSV Field Separator if CSV has been chosen as the input format. It should be hidden otherwise as it's confusing.

Comments

turadg’s picture

turadg commented
Assigned: Unassigned » turadg
Status: Active » Needs review

Committed to 4.x dev. Please review.

sivaji_ganesh_jojodae’s picture

sivaji_ganesh_jojodae commented

There is a typo in your JQuery statement. The import format is set to "Comma separated value" when the page loads but your jQuery statement does not attempts to shows the field separator text box. Hide/Show works only when the select item changes. Here the fix

/**
 * supporting Javascript code for questions_import module
 */
$(document).ready(function() {
  // show separator field if the selected value is "native csv"
  if ($("#edit-import-type option:selected").val() === 'native_csv')
    $('#edit-field-separator-wrapper').show();
  // invoke these lines when the list selected item changes
  $('#edit-import-type').change(function() {
    // show separator field if the selected value is "native csv"
    if ($("#edit-import-type option:selected").val() === 'native_csv')
      $('#edit-field-separator-wrapper').show();
    else
      $('#edit-field-separator-wrapper').hide();
  });
});
turadg’s picture

turadg commented
Status: Needs review » Fixed

That wasn't a typo. That was a commit-o.

I forgot to include my edit to questions.admin.inc that put Aiken first instead of CSV. I thought the alphabetical order would be more clear.

Your code is more robust in that it can handle CSV being first or not, which I think is an improvement.

I had used the text name and searched for "Separator" in case there were other import types that have separators, (e.g. Moodle CSV) but I agree it's better to use the option val() instead, in the case of localization. We could search for "csv" in the val() string to accommodate non-native csv, but that can be done later if it's ever necessary.

thanks for the review.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.