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

Rewrite Meetup class #11

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sukima
Copy link

@sukima sukima commented Nov 6, 2015

This is more JavaScript class idiomatic. It also allows for future abstractions like removing the hard coded URLs.

The original file used tabs so I kept that even though the .vimrc seems to contradict this.

This also uses UMD for encapsulation which makes it more universal to be pulled into other websites.

@sukima sukima force-pushed the feature/Meetup-object-redux branch from 898a18d to 04de873 Compare November 6, 2015 02:17
@sukima sukima force-pushed the feature/Meetup-object-redux branch from 770717b to b46066d Compare November 6, 2015 02:35
@sukima
Copy link
Author

sukima commented Nov 7, 2015

This is ready for review/merge. Next step is to pull the constants EVENTS_URL and MEMBERS_URL out of the code and into some config. But that can be for another pull request.

@sukima
Copy link
Author

sukima commented Nov 7, 2015

As an aside… might want to consider using a package manager for vendor scripts like bower or npm.

This is more JavaScript class idiomatic. It also allows for future
abstractions like removing the hard coded URLs.

The original file used tabs so I kept that even though the .vimrc seems
to contradict this.

This also uses UMD for encapsulation which makes it more universal to
be pulled into other websites.
window.onload will overwrite and isn't really the best place since it
doesn't take into account when the DOM is ready. We have jQuery so use
it to initialize correctly.
@sukima sukima force-pushed the feature/Meetup-object-redux branch from 9e02617 to aa238b4 Compare November 8, 2015 18:55
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.

1 participant