-
Notifications
You must be signed in to change notification settings - Fork 5
org/listeners #200
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
org/listeners #200
Conversation
|
@TrevorBurgoyne @csolbs24 let me know what you think. This doesn't change any functionality so I'm not that hurried to get this merged. If I can get a couple other organizational PRs together perhaps it would be nicer to bundle them. |
|
Marking this as "ready for review" as #201 and #190 are dependent on this (in that order) and there doesn't appear to be any lost functionality in either of those branches. Fine with bundling all of them together into #190 as this and #201 are basically just organizational setup for it. I don't think it matters as much with these pre-1.0.0 releases but I'd like to at least get familiar with bundling PRs together into a single release just to have that skill under our belts. |
TrevorBurgoyne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks g, everything still seems to work as well 👍
Pull main event listeners into their own file
Description
As mentioned here,
ULabelhas static methods that take a class instance, and calling those gets problematic. This PR breaks out event listeners and the night mode cookie.listeners.tscreate_ulabel_listeners- called duringULabel.initremove_ulabel_listeners-ULabel.remove_listenerscalls this but is kept for conveniencecookies.tsNightModeCookieindex.jsis 400 lines shorter which is a win. Passing around the wholeULabelobject isn't ideal but this is a step in the right direction.PR Checklist
package.jsonhas been bumped since last releasepackage.jsonandsrc/version.jsnpm installandnpm run buildAFTER bumping the version numberapi_spec.md)changelog.mdBreaking API Changes
If anyone was explicitly calling
ULabel.create_listenersrather than lettinginitdo it, that functionality is gone.