Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Spinner: use data attributes when initialising indirectly #1206

Merged
merged 1 commit into from
May 6, 2015

Conversation

jwarby
Copy link

@jwarby jwarby commented Apr 1, 2015

If the options for a Spinner are declared as data attributes, the data attributes are only used during instantiation if the Spinner is initialised via the user performing a mousedown event on the element (data attributes API).

If you call a Spinner method on an element prior to instantiation, the Spinner will get instantiated but the data attributes aren't passed through.

I have a use case where my Spinner starts off in a disabled state, and I enable it later if the user performs a certain action. In this case, the data-min and data-max values from my markup don't get applied because the call to disable initialises the Spinner without using the data attributes. Then, when I enable the Spinner later, I am able to go below data-min and above data-max (the default limits of 1 and 999 are applied instead). This JSFiddle demonstrates the issue. I haven't checked to see if any of the other plugins have the same issue.

instantiating spinner by calling a spinner method on an element
@interactivellama interactivellama added this to the 2.6.6 milestone Apr 11, 2015
@interactivellama interactivellama removed this from the 2.6.6 milestone Apr 11, 2015
@interactivellama
Copy link
Contributor

@jwarby I understand your issue here when starting with a disabled spinner and therefore invalid data from the user to set a max and min. However, we have a history in this library about initialization. This is from the Fuel UX 3 documentation, but I believe the ability to update the options when initialized a second time should not be present in any of our controls.

Fuel UX controls cannot be re-initialized. Passing new options to an already initialized control will do effectively nothing. Many controls have convenience methods that allow some internal variables to be changed; otherwise use Javascript to call the control's destroy() method and initialize again with the newly desired markup and options. The destroy() method will attempt to return the pre-initialization markup for the element being destroyed.

Would it be possible to replace the disabled spinner control in the DOM with new un-initialized markup?

Forgive me if I am misunderstanding you. @kevinparkerson Do you have any additional thoughts? You are more familiar with version two.

@jwarby
Copy link
Author

jwarby commented Apr 11, 2015

@interactivellama I think there is a little bit of confusion, which is probably me not explaining properly sorry.

In my use case, I have set data attributes for min, max and the disabled state. When I call the enable method, I am expecting the spinner to already be initialised with my data attributes but it isn't. The call to enable the spinner has the side effect of initialising the spinner before being applied. With my fix, the side effect-initialisation makes use of the DOM data attributes.

As far as I can tell, my fix won't allow you to re-initialise the spinner with different options because there is a check on line 206 which checks if the spinner is already initialised (if (!data) { ... }).

Does that make any more sense....?

@interactivellama interactivellama added this to the 2.6.6 milestone Apr 17, 2015
@swilliamset swilliamset self-assigned this Apr 17, 2015
swilliamset pushed a commit that referenced this pull request May 6, 2015
Spinner: use data attributes when initialising indirectly
@swilliamset swilliamset merged commit 4876a35 into ExactTarget:fuelux2 May 6, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants