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

Template project? #54

Closed
nemoo opened this issue Dec 19, 2014 · 14 comments
Closed

Template project? #54

nemoo opened this issue Dec 19, 2014 · 14 comments
Labels

Comments

@nemoo
Copy link

nemoo commented Dec 19, 2014

Is there a sbt template project available on gibthub that i can just check out and run in order to get started quickly with scalajs-react?

@japgolly
Copy link
Owner

Nah. The setup is trivial though and documented in the README. If you prefer looking at something already setup you'll probably be able to find other projects on github using scalajs-react. There's this project's gh-pages module too that might be similar to what you're looking for.

If someone wants to create a template project, feel free and I can open this issue to track it and help out if needed.

@chandu0101
Copy link
Contributor

@japgolly

If someone wants to create a template project, feel free and I can open this issue to track it and help out if needed.

https://github.com/chandu0101/scalajs-react-template

please review it :)

@oyvindberg
Copy link
Contributor

@chandu0101 could we flatten those package names a bit? how about just «template»?

also I've written something you could consider a template - for todomvc.com:
tastejs/todomvc#1323

@chandu0101
Copy link
Contributor

sure , changed to scalajsreact.template ( intellij didn't allowed me refactor to just template)

@oyvindberg
Copy link
Contributor

hey @chandu0101 , would you mind if we did some mutual review of these two projects?

I want some code in todomvc so people can easily compare what we're doing here with other ways, and they of course want some experienced people to have a look to confirm that i'm not doing anything stupid.

or could you lend a hand, @japgolly ?

still talking about tastejs/todomvc#1323 (comment) that i mentioned above

@chandu0101
Copy link
Contributor

@elacin

hey @chandu0101 , would you mind if we did some mutual review of these two projects?

sure mate , to be honest i am still learner in react/scala , lemme see if i can find something useful! :)

val model = new TodoModel(Storage(dom.ext.LocalStorage, namespace))

i see this created for every route access! , can we move this to Main file as its constant for whole app!

 val routerM = React.render(router, dom.document.getElementById("todoapp"))

why its assigned to routerM ? :s

tomorrow i'll give closer look , Mean while feel free review mine :)

@japgolly japgolly reopened this Jun 7, 2015
@japgolly
Copy link
Owner

japgolly commented Jun 7, 2015

I'll create some links to these and @ochrons's SPA tut as well.

Disclaimer: I only skimmed through both of your stuff. I'll take your word that it all actually works :)

@chandu0101 I don't agree with your organisation - things are spread all over the place for no benefit in my opinion. That's my only real feedback. Everything else looked fine enough. I was interesting to see how to do styles - it's quite different than how I do them. I have one styles object for my entire app. It has inner objects for organisation. Interesting to see a different approach.

@elacin

  • Your indenting kills me. You could measure it in metres.
  • You're using unicode all over the place. Personally I don't have any problem with that. I use linux with xcompose, I can easily type π ≠ ⇒ ∈ ↖ ☒ ᵉᵗᶜ but lots of people in OSS get really agitated when they see it. If this is to entice people to a cool way of writing webapps the message may get lost amongst reactions to the unicode arrows everywhere. Plus I'll guess that windows probably won't even display half of it.
  • I see two stateful components. It's best to minimise state to a single component, preferably a dumb top-level component that does nothing but wire up state and connect components.
  • Where you use backends I find it much nicer (especially on IDEs) to move the render function into the backend as well.
  • You're using Broadcaster and there seems to be some custom logic to refresh a component. I recommend you use Listener in conjunction with Broadcaster. Have a look at router2.Router for an example. Also there a little snippet in extra/README.md.
  • Looking at your state class, you have a few .mod(.map.(copy. Monocle lenses would simplify this.

@chandu0101
Copy link
Contributor

@japgolly

@chandu0101 I don't agree with your organisation - things are spread all over the place for no benefit in my opinion.

thanks for your time mate , if my guess is correct you're not happy with routes package! , that being said love to know what you have in your mind :)

@japgolly
Copy link
Owner

japgolly commented Jun 7, 2015

The way I look at it, if you have a component that has a backend, props,
model, state.. - if all of that stuff exists only for the one component,
then I put it all together. One object with a component, props, backend etc
all inside of it because it all comprises one logical unit, even if you can
technically pull it apart. I don't think it's a good idea to pull things
apart because you can, if it doesn't make sense to do so. (This is after a
few years & projects of me doing it exactly the way you did (cos it does seem
like a good idea)).

The routes I consider separate because the component or "feature" doesn't
need a special route to work. Routes are a concern of a single-page so I
when I write a single-page I group all routes it cares about and creates,
together in one object, along with the router for it. Ergo I wouldn't have
a routes package, just a single object in the same package as the
entrypoint/JSApp.

Now I'm a bit of a hypocrite (call me a lying bastard!) because contrary to
my first paragraph I put all of my styles into a single Style object.
Said object has inner objects for organisation. It feels like the right
thing to do but it did jump out logically as I wrote paragraph above. I
haven't thought about why I feel this is an appropriate exception. It was
interesting to see you take a different approach to styles. I'd like to see
more and hear more stories over time. So far though, illogical as it may
be, having all my styles together has felt consistently great. :)

On 7 June 2015 at 11:08, Chandra Sekhar Kode notifications@github.com
wrote:

@japgolly https://github.com/japgolly

@chandu0101 https://github.com/chandu0101 I don't agree with your
organisation - things are spread all over the place for no benefit in my
opinion.

thanks for your time mate , if my guess is correct you're not happy with
routes package! , that being said love to know what you have in your mind
:)


Reply to this email directly or view it on GitHub
#54 (comment)
.

@chandu0101
Copy link
Contributor

The way I look at it

class!, that sums up all . organizing code is subjective it just reflects perspective of dev nothing more!

styles : I see scalacss as pure inline styles with magic of all css support(pseudo selectors ,media queries, prefix ,...) , apart from that when ever i start a new component (not container components) i always think it as a reusable component ( how many end up as real reusable components is a question though :p) thats why i place styles near to component def so that i can easily lift them to a different projects.

Note : thank you for making scalacss happen ,lots of love, cheers 👍

@oyvindberg
Copy link
Contributor

Thanks a lot for your time , @japgolly .

  • Agree completely about indenting, but cannot do anything about it i think (todomvc project demands unholy tabs instead of spaces)
  • Also agree completely about unicode arrows, have taken them out.
  • Regarding Broadcaster/Listener i remember ending up with that hack due to Listener being hard to grok/use. I will be sending you a proposal for an API which drives type inference so it's easier to use. Looking forward to your comments on that. For now i have fixed it like the examples you quoted.
  • i will keep monocle out of this for now, it's already enough concepts in here for a supposedly beginner friendly example, imho.

That leaves your comment about state in two places. I really thought it was more logical the way I did it, but will experiment with it.

@japgolly japgolly added doco and removed question labels Jun 8, 2015
@japgolly
Copy link
Owner

japgolly commented Jun 8, 2015

Np & thanks to both of you. Rock on and I'll link to your stuff when I next work on scalajs-react documentation. 👍 👍

@japgolly
Copy link
Owner

Documented in 6a0817f
Thanks guys

@oyvindberg
Copy link
Contributor

Cool. I need to follow up to actually have that PR merged, will update that link for you when that happens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants