Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upImplement `ampersand-app` for ampersand CLI #91
Conversation
mikehedman
reviewed
Jan 27, 2015
| // all the <a> tags in the app. | ||
| // it expects a url without a leading slash. | ||
| // for example: "costello/settings". | ||
| navigate: function(page) { |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
mikehedman
Jan 27, 2015
Contributor
I'd kind of hoped that this method would get "promoted" into ampersand-app. It seems like pretty much every ampersand application would need it, so it's always seemed wrong for it to have to be copy/pasted into the main app file. Now that there's a parent, it seems like a great place to put it.
But then again, it would trigger promotion of the router. But again, if all apps need a router, navigate, and domReady function... You know, let's turn Ampersand into a "frameworky" framework :(
mikehedman
Jan 27, 2015
Contributor
I'd kind of hoped that this method would get "promoted" into ampersand-app. It seems like pretty much every ampersand application would need it, so it's always seemed wrong for it to have to be copy/pasted into the main app file. Now that there's a parent, it seems like a great place to put it.
But then again, it would trigger promotion of the router. But again, if all apps need a router, navigate, and domReady function... You know, let's turn Ampersand into a "frameworky" framework :(
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
HenrikJoreteg
Jan 27, 2015
Member
@mikehedman I think there's some more we can do here while still maintaining flexibility.
I'm going to tweak some of this a bit and also go add some tests and such. Thanks for doing the initial conversion @fyockm. More thoughts incoming...
HenrikJoreteg
Jan 27, 2015
Member
@mikehedman I think there's some more we can do here while still maintaining flexibility.
I'm going to tweak some of this a bit and also go add some tests and such. Thanks for doing the initial conversion @fyockm. More thoughts incoming...
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
fyockm
Jan 27, 2015
Member
@mikehedman you make some very good points on both sides. I personally don't think ampersand-app (at least in its current incarnation) is the correct place for this abstraction, but perhaps there is place for this "boilerplate" type code?
I'll be interested to see what @HenrikJoreteg and crew have to say on the topic.
fyockm
Jan 27, 2015
Member
@mikehedman you make some very good points on both sides. I personally don't think ampersand-app (at least in its current incarnation) is the correct place for this abstraction, but perhaps there is place for this "boilerplate" type code?
I'll be interested to see what @HenrikJoreteg and crew have to say on the topic.
bear
added
enhancement
request
labels
Jan 27, 2015
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
HenrikJoreteg
closed this
Feb 14, 2015
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
HenrikJoreteg
Feb 14, 2015
Member
Let's continue this discussion in #96. I just made a few tweaks that I was envisioning there, @fyockm and @mikehedman.
|
Let's continue this discussion in #96. I just made a few tweaks that I was envisioning there, @fyockm and @mikehedman. |
fyockm commentedJan 27, 2015
as mentioned in AmpersandJS/ampersand-app#4