Skip to content

Components - #87

Merged
timwis merged 9 commits into
gh-pagesfrom
components
Apr 14, 2016
Merged

timwis merged 9 commits into
gh-pagesfrom
components

Conversation

@timwis

@timwis timwis commented Apr 10, 2016

Copy link
Copy Markdown
Owner

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
Copy Markdown
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

timwis commented Apr 11, 2016

Copy link
Copy Markdown
Owner Author

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

@timwis

timwis commented Apr 11, 2016

Copy link
Copy Markdown
Owner Author

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
Copy Markdown

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

timwis commented Apr 11, 2016

Copy link
Copy Markdown
Owner Author

@themightychris

Copy link
Copy Markdown

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

JJediny commented Apr 12, 2016

Copy link
Copy Markdown
Contributor

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

timwis commented Apr 14, 2016

Copy link
Copy Markdown
Owner Author

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