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

Components #87

Merged
merged 9 commits into from
Apr 14, 2016
Merged

Components #87

merged 9 commits into from
Apr 14, 2016

Conversation

timwis
Copy link
Owner

@timwis timwis commented Apr 10, 2016

This pull request involves a complete refactor of the javascript architecture. Previously, JKAN used a router-like configuration to determine page page it was on, and then executed logic for that particular page. Now JKAN looks for a list of components on any page and initializes each component class (see index.js), passing it its container element and any shared global variables. Components are configured through data-xxx="" attributes on the element (where configuration is necessary). This allows components to be reused on any page.

My hope is that this is easier to dive into and more reusable. Thoughts @pezholio @themightychris?

Might be easier to view by looking at the branch.

EDIT: I've also documented it on the Architecture wiki page.

Closes #78 and closes #85 and closes #66

@timwis timwis mentioned this pull request Apr 11, 2016
@pezholio
Copy link
Collaborator

Ah, smart. Also makes things a lot more DRY too. I think I might've struggled with this a bit more initially - JS isn't my first language, and I think the MVC-ish nature of the router approach meant it made sense to me. That said, if this was documented, I'd be able to dive into this easily 👍

@timwis
Copy link
Owner Author

timwis commented Apr 11, 2016

Thanks @pezholio - I just took a stab at documenting it in the wiki.

@timwis
Copy link
Owner Author

timwis commented Apr 11, 2016

Pinging @JJediny @abhinemani @Floppy as other folks who've dove into JKAN's code base. Do you also feel this makes sense? Any reason not to merge?

@themightychris
Copy link

Some docs outlining what the runtime flow is and how a developer would find where the code for something is and how to add components would be a great help, reading the code only have me a partial fuzzy picture of it. I wouldn't hold off merging for that though it's not a regression over what was before

@timwis
Copy link
Owner Author

timwis commented Apr 11, 2016

@themightychris
Copy link

I did not, that's perfect! (and as a side note I'm a proponent of versioning docs alongside the code they document so it's never fuzzy what versions of docs and code go together)

@JJediny
Copy link
Contributor

JJediny commented Apr 12, 2016

After taking a look at the file changes/wiki/thread i see no reason not to pull. The more abstracted the JS is from Jekyll the easier it will be to reuse components else where so +1...

Without knowing the full effect, is there any functionality lost?

@timwis
Copy link
Owner Author

timwis commented Apr 14, 2016

Thanks for the feedback guys. and @JJediny nope, no functionality lost.

@timwis timwis merged commit 5128d8d into gh-pages Apr 14, 2016
@timwis timwis deleted the components branch April 14, 2016 09:53
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.

Remove setup page from JKAN core Component-based JavaScript architecture Use growl-style alerts
4 participants