Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve configuration loading issues #1

Merged
merged 2 commits into from
Oct 13, 2017

Conversation

luejerry
Copy link
Contributor

@luejerry luejerry commented Oct 12, 2017

Resolves issues with saved configurations not being properly applied on load. The root cause was race conditions occurring in two places:

  1. Checkbox click handler execution occurring before checked state is toggled in the DOM
  2. Event handler attachment to elements before they are added to the DOM

Race condition 1 was resolved by changing use of jQuery click() to change() handlers for checkboxes, which is guaranteed to fire after the DOM element is toggled.

Race condition 2 was resolved by ordering the addition of the main menu DOM elements to occur before associated event handlers are attached. This also removes the need to load config with a timeout.

This issue was caused by logic of click() handlers on checkbox elements, which produces different results depending on whether the DOM element toggle or click handler is executed first. Resolved by using change() handlers instead.
Timeout allowed config values to load because without it, a race condition could occur where events get fired on elements before they are defined. Resolved by ordering DOM element appends before config loading.
@Alice-Cheshire Alice-Cheshire merged commit 5257b27 into Alice-Cheshire:master Oct 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants