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

Hide width flickering when rendering to DOM is delayed #621

Closed
wants to merge 1 commit into from

Conversation

jrbalsano
Copy link

This is a continuation of #516. The original description is copy-pasted below:

This pull request makes two changes to reduce the problems described #478.

  1. The length and methods of timeouts have been changed to reduce the time spent in the intermediary states described in switch flicker in v3.3.2 #478.
  • Reducing the timeout interval to 0 in _containerPosition: This change leaves the timeout in place to allow the callback to be called after a browser repaint. This means that in _init's case, animations won't be applied to the element when the browser initially adjusts the width, but as soon as the width has been adjusted _init's callback is called and the animate class is applied to the switch if necessary.
  • Changing the interval in _init to use an exponential backoff: This change reduces the timeout to 0ms initially but doubles it every time we check so that the browser isn't left polling consistently at 50ms. The starting value allows for the possibility that the element will be inserted to the DOM right after bootstrapSwitch is initialized, while the exponential backoff ensures polling doesn't kill the browser.
  1. The switch is now hidden if it was not :visible from the outset. This is to prevent the user from seeing the intermediary stages as described in switch flicker in v3.3.2 #478. This solution was described by @effiek9999 and I have modified it slightly to only be applied in the situation when timeouts are brought into play. I also got rid of the fadeIn effect @effiek9999 proposed to not opt users into undesired display behavior. Hopefully the changes describe in 1 have reduced the need for this.

I had since updated and left a comment:

Updated! I was a little bit unfamiliar with the overall changes to the flow that seem to have occurred in the meantime but I think these changes have the desired results. The only difference seems to be that the interval in _containerPosition is now also inside the _init method.

dist/js/bootstrap-switch.js Outdated Show resolved Hide resolved
src/js/bootstrap-switch.js Outdated Show resolved Hide resolved
@graingert
Copy link
Collaborator

Can you also write a test for this change

@graingert
Copy link
Collaborator

@jrbalsano ping

@jrbalsano
Copy link
Author

Hey y'all, sorry for the delay on this but I was caught a little off-guard with the requests after the PR had been sitting there for months. I tried to accommodate then, but the extra requests weren't something I had time for.

I got back into it today and I've update to respond to your comments. However, I think I'm having a little trouble writing a test for this change. If one of you would be willing to work with me on that aspect it would be much appreciated.

@LostCrew LostCrew self-assigned this Apr 3, 2018
@LostCrew
Copy link
Member

Inactivity

@LostCrew LostCrew closed this Apr 13, 2019
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

3 participants