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

Remove unused loadEvent property in ControlBar options #3363

Closed
wants to merge 1 commit into from

Conversation

m14t
Copy link
Contributor

@m14t m14t commented Jun 9, 2016

Description

ControlBar's default options currently contains a misleading loadEvent property.

Looking through the code (grep -irl loadEvent --exclude-dir=node_modules *) shows that this property is not used anywhere.

Specific Changes proposed

Remove an unused property.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors

@gkatsev
Copy link
Member

gkatsev commented Jun 9, 2016

LGTM.
@heff do you remember why the controlbar even had a loadEvent?

@gkatsev gkatsev added patch This PR can be added to a patch release. needs: LGTM Needs one or more additional approvals labels Jun 9, 2016
@heff
Copy link
Member

heff commented Jun 10, 2016

It's a relic. Something where it wouldn't initialize until the play event happened (after the big play button was clicked). Can't believe it made it through all 5.0 work. It's a survivor! :feelsgood:

@gkatsev gkatsev added confirmed and removed needs: LGTM Needs one or more additional approvals labels Jun 17, 2016
@gkatsev
Copy link
Member

gkatsev commented Jun 28, 2016

merged to stable. Will be closed when released and merged to master.

@gkatsev gkatsev closed this in 7f6ce63 Jun 28, 2016
@m14t m14t deleted the feature/remove-load-event branch July 31, 2018 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed patch This PR can be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants