tableau-mkt / jquery.addressfield Goto Github PK

Hi, when trying to implement this library through NPM, we keep getting an error
Uncaught ReferenceError: jQuery is not defined

We added these fields to our code
const $ = require('jquery') require('jquery.addressfield')

and JQuery is a dependency in our package lock

"jquery": { "version": "3.4.1", "resolved": "https://registry.npmjs.org/jquery/-/jquery-3.4.1.tgz", "integrity": "sha512-36+AdBzCL+y6qjw5Tx7HgzeGCzC81MDDgaUP8ld2zhx58HdqXGoBd+tHdrBMiyjGQs0Hxs/MLZTu/eHNJJuWPw==" }, "jquery.addressfield": { "version": "1.2.3", "resolved": "https://registry.npmjs.org/jquery.addressfield/-/jquery.addressfield-1.2.3.tgz", "integrity": "sha1-7SaUISfKOUsbqx6zTJKU7bT9Y6c=", "requires": { "addressfield.json": "github:tableau-mkt/addressfield.json#1.1", "jquery": ">=1.3.2" } },

We of course thought it might be our own setup, but when running the library through the RunKit link on the npm page (https://npm.runkit.com/jquery.addressfield), we experience exactly the same error at the same spot in the code.

ReferenceError: jQuery is not defined

jquery.addressfield/dist/jquery.addressfield.js — line 19

Is this library still maintained and useable?

Thank you,

Terry Fox

Looks like IE8 does a stupid thing where, for dynamically created elements, the name attribute on a DOM object is element.propdescname rather than element.name.

"Real" fix would probably be a change to the way we read all attributes off of a given element, but the quick fix is to just add it to the list of properties to check (IIRC, the attribute copy method is written in such an odd way for compatibility anyway).

This affects both 0.2.x and 1.x. Patch should be applied to all.

I had a great deal of frustration trying to use this plugin, until I realized that I could make my .locality group stop disappearing if I wrapped label-input pairs in a container (e.g. div.form-group). The usage docs could use some explicit detail about the requirements for marking up the form.

What is the best way to set which country should be selected on page load? Do I just write my own JS to do this? If I just need to set the selected element after load, is there a method that I can have it redraw the fields?

I noticed you're using for ... in to iterate over the properties in the config object.

A for...in loop iterates over the properties of an object in an arbitrary order (see the delete operator for more on why one cannot depend on the seeming orderliness of iteration, at least in a cross-browser setting).

Seems like you might want to refactor or warn users that it's not a guaranteed order.

Should keep the README lean, just keep the features highlights + installation + basic usage. Then, in a "docs" folder, create topic-based documentation, linked to from the README:

• Detailed usage: goes into detail about format of config and expected structure of the form.
• Overridding functionality: goes into the methods in the plugin that can be overridden.

The releases are triggering weird things on the jQuery plugin page... Better to just do it manually. Not that hard.

Would be nice if you could set the administrative area the same way you could with the Country data attribute.

The addressfield.orderfields method uses .class, which is a protected property in IE8.

We can work around this by accessing it using array notation.

// From...
$element.find('.' + order[i].class).val(order[i].value).change();

// To...
$element.find('.' + order[i]['class']).val(order[i].value).change();

I'm planning on having a browserified build that includes this as a dependency. There's a package.json in the project root, so it seems like this would work via an NPM install. Looking at package.json, it looks like plugin has been registered on NPM with "jquery-plugin".

npm installing that gives me the package, but I think there are a few issues here:

1. jquery and addressfield.json aren't listened as runtime dependencies, so I don't get them with the install (unlike if I install this with bower). node will not find these dependencies at runtime, so this will likely break on a browserified build that happens to not include jquery as a separate dependency.
2. in bower.json, I see the main file as dist/jquery.addressfield.js. I don't have this however:

example $ ls -lh node_modules/jquery-plugin/dist/
total 16
-rw-r--r--  1 valvarez  TSI\Domain Users   842B Sep  7  2012 foo.js
-rw-r--r--  1 valvarez  TSI\Domain Users   435B Sep  7  2012 foo.min.js

3. this is more of a suggestion, but could we make the package name on NPM a bit more specific, something like "jquery.addressfield" instead of "jquery-plugin"?
4. I get development files in the distribution:

example $ ls -lh node_modules/jquery-plugin/libs/
total 8
drwxr-xr-x  3 valvarez  TSI\Domain Users   102B Sep  7  2012 jquery
-rw-r--r--  1 valvarez  TSI\Domain Users   506B Sep  7  2012 jquery-loader.js
drwxr-xr-x  4 valvarez  TSI\Domain Users   136B Jan 17 19:33 qunit
example $ ls -lh node_modules/jquery-plugin/src/
total 8
-rw-r--r--  1 valvarez  TSI\Domain Users   851B Sep  7  2012 foo.js
example $ ls -lh node_modules/jquery-plugin/test/
total 16
-rw-r--r--  1 valvarez  TSI\Domain Users   1.1K Sep  7  2012 foo.html
-rw-r--r--  1 valvarez  TSI\Domain Users   1.7K Sep  7  2012 foo_test.js

I also get a foo.jquery.json in my project root. Do I need these on my disk?

Proposed solutions:

• exclude unneeded files in .npmignore
• rebuild and republish to npm
• change npm package name
• enumerate jquery and addressfield.json in dependencies or peerDependencies

This issue is occurring for us in Drupal with scale_addressfield, but the root is here.

We cannot use multiple address fields on the same page with jQuery versions 1.7, 1.8, 1.9 or 1.10 (we stopped testing there).

When we have two addressfields on screen, and are using one of the jQuery versions above we get:

Uncaught TypeError: Cannot read property 'fields' of undefined

I traced this back to here.
In this method this.value is 0 because the context is not strict enough. As a result of the error, the State fields never becomes dropdowns because the bindings were never assigned.

jQuery 1.5 is fine, and all versions are fine with only a single address field.

A simple fix, incomming.

We've abstracted things into addressfield::hideField and addressfield::showField, but we still manually use the :visible selector in the main plugin method.

This will cause the plugin to break In cases where end-users have overridden the hide/show methods to match their markup.

Solution is to add an addressfield::isVisible method that matches the assumptions of our show/hide methods and use it in the main plugin instead of the :visible check.

The 1.x addressfield.json configuration schema provides for a "format" property on field configuration objects, representing a regex pattern for validation:

{
  "label": "State",
  "options": [],
  "format": "^[a-zA-Z]{2}$"
}

We should provide (optional) integration with a form validation framework based on any provided values. Probably https://github.com/jzaefferer/jquery-validation.

Running a test results in the following:

Running "qunit:all" (qunit) task
Testing test/addressfield.html?jquery=1.3.2 
PhantomJS threw an error:ERROR
>> 0 [ '' ]
Warning: PhantomJS exited unexpectedly with exit code null. Use --force to continue.

Aborted due to warnings.

According to this thread on qunit, the suggestion is to move away from grunt-contrib-qunit and PhantomJS and use another testing framework instead.

You might want to try using Karma to run the tests in a real browser (e.g. Chrome or Firefox). grunt-contrib-qunit and PhantomJS are fairly old and might not give a realistic result for end-users.

Consider packaging addressfield.json and optionally using it by default, but allow override to either another JSON file, or provide the config inline.

Replace this:

$.addressfield(config, enabled_fields);

With something like this:

$.addressfield({
  // Inline configuration, no async JSON fetching.
  config: {}

  // Or, defaults to pulling this async via JSON:
  json: 'addressfield.json',

  // Rather than a simple list of fields, maybe a way to specify selectors for each...
  // Or maybe just maintain the existing structure and save the update for another day.
  fields: {
   'postalcode': '#postfield',
   'locality': '#foo .locality-name'
  },

  // Will maybe need to start providing this too:
  country_field: 'select#country'
});

In the licence of it says this jquery plugin is MIT however in the javascript file it is stated as being GPL 2.0. Does this just need to be changed in the javascript file or is it actually GPL 2.0?

The plugin attaches a custom method name in the format 'isValid_' + field to each element on the form. This is particularly useful for postal codes, since the regexp validation for each is different per country.

When there are two postal code fields on the same page (in different forms), the second form's postal code validation seems to apply to the first as well. So if I select "Canada" in my second form, and leave the first form as "United Kingdom", the "United Kingdom" postal code will be validated using the "Canada" postal code rules. This is because we use the same method name for the validation rule, hence the conflict. They are both named "isValid_postalcode".

Should probably make this method name depend on the country, or a hash of the config array. Something along those lines.

Cannot get basic config to work. Consider this example(Mostly taken from docs):

<html>
<head></head>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<script src="jquery.addressfield.js"></script>
<body>
  <form id="myForm">
    <div class="field-wrapper">
      <div class="field-wrapper">
        <label for="country">Country</label>
        <select id="country"></select>
      </div>
    </div>
    <div id="locality-fields">
      <div class="field-wrapper">
        <label for="city">City</label>
        <input type="text" id="city" />
      </div>
      <div class="field-wrapper">
        <label for="state">Administrative Area</label>
        <input type="text" id="state" />
      </div>
      <div class="field-wrapper">
        <label for="zip">Postal Code</label>
        <input type="text" id="zip" />
      </div>
    </div>
  </form>

  <script type="text/javascript">
  $('#myForm').addressfield({
    json: 'addressfield.json',
    fields: {
      Country: '#country',
      locality: '#locality-fields',
      localityname: '#city',
      administrativearea: '#state',
      postalcode: '#zip'
    }
  });
  </script>
</body>
</html>

addressfield.json is the version which is distributed with addressfield.js. Also note that I needed to capitalize 'Country' in order to avoid the error :

"Uncaught TypeError: Cannot read property 'fields' of undefined"

I have confirmed that addressfield.js and addressfield.json are available and properly accessed. Running this html page results in a form with an unpopulated country dropdown and no functionality.

Would be cool to package this up into a web component. If I have time this week, I'll give it a shot.

Does this plugin support localization of the fields?

Both the convertToSelect and convertToText methods make assumptions about the form element attributes that should be maintained when converting element types.

They're sensible defaults, but we should allow others to add their own (or get rid of the defaults, if need be).

A configuration layer is beyond a 0.2.x release, so maybe we should just abstract it into an override-able method.

Currently this plugin doesn't appear on bower search.

Would be nice to be able to bind event handlers for when addressfield has done its thing.

Some methods are extremely opinionated in finding form element containers, and they aren't even all necessarily of the same opinion.

The showField/hideField methods assume the parent element. The orderFields method assumes the parent, but specifically a div or section...

We should probably introduce a generic elementContainer method that finds a form elements containing element. The definition of which is something along the lines of... "The closest element that contains both this form element and its label"

We should then update showField, hideField, and orderFields to use this new method.

Trying to use jQuery validate with the plugin and getting an error

Uncaught TypeError: Cannot read property 'settings' of undefined

Error is starting here

// Account for nested fields. if (config.fields[fieldPos][field] instanceof Array) { return $.fn.addressfield.apply.call($element, {fields: config.fields[fieldPos][field]}, fieldMap); }

Here is how I am instantiating addressfield

$('#contact-form').addressfield({ json: 'bower_components/addressfield.json/src/addressfield.json', fields: { country: '#Country', locality: '#locality-fields', administrativearea: '#State', postalcode: '#Zip' }, });

Thoughts

The updateLabel method is extremely opinionated about the location of the label relative to the form element.

Rather than assuming that the previous element is a form element's label, we should explicitly check the label's for attribute matching the form element's name.

For API compatibility reasons, we may want to keep the existing prev check in an elseif.

I have a field that is converted from an input field to a select and the data attribute is not being carried over.

@Html.TextBox("Address.State", @Model.Address.State, new { @class = "input select-state", @id = "State", data_required = "true", data_selected_state = Model.Address.State })

Would be a cool feature enhancement to support phone number formatting. Thoughts? I could look at adding feature if you need someone to do the work.