Make routing easier

[misstick]

Enhancement

Remove panels layout

• [x] Remove panel_router(useless abstraction),
• [x] remove firstPanel and secondPanel ,
• [x] remove router_panel.coffee
• [x] remove component/panel.coffee
• [x] clean Views : add getters for 1rst level views (application, Panel)
• [x] less views are listening to Stores changes

  Easier routes

• [x] make Routes easier,
• [x] move filtershandling into getter/router,
• [x] redirection : do not redirect into viewsbut with routerActionCreator.navigate,
• [x] use real DOM links to change Route directly,
• [x] clean buildURL parameters + change its calls,
• [x] no window.location.href into views.

  Clean actionCreators

• [x] remove callback into actionsCreator,
• [x] actionsCreator can't call other actionsCreator,
• [x] remove callback to be executed after dispatch (anti-pattern)

  More smaller stores

• [x] move notifications from layout_store to notification_store
• [x] create selection store

  Features concerned

Check new routes for :

• [x] new Account,
• [x] edit account,
• [x] Message List,
• [x] Message display,
• [x] Message edit,
• [x] Message reply,
• [x] Message reply all,
• [x] Message forward,

  Synthese

• [x] Make schemas to explain : before, after and what next?
• [x] Write guidelines to use Router, and redirection int the App

  Fix features

• [x] Selection or mailbox item into left Menu,
• [x] Menu can't be minimized,
• [x] remove fullscreen button/mode,
• [ ] keyboard navigation
• [x] each message of a conversation has got its own URL

[frankrousseau]

@misstick Is it possible to merge that PR and open a new one with the remaining of the todo-list?

[misstick]

@frankrousseau : no it is not finished yet, and it need to be reviewed

[frankrousseau]

Yep but I don't like huge PR. That one will be close to non reviewable because of its size.

[misstick]

@frankrousseau : yes it can't be split in other PR : this feature has too much consequences to be smaller

[m4dz]

@frankrousseau this PR concerns the core routing that also impacts stores, and I can't see a clean way to split it without side-effects. We made a meeting with @poupotte and @aenario this morning, we'll see what we can do to fix it :).

[frankrousseau]

Ok @misstick and @m4dz

[frankrousseau]

Thx for the info update.

[m4dz]

@frankrousseau no problem :). We'll merge it after pre-review (from me and @aenario) tomorrow, and then use it as a solid base structure for the rest of the app. It'll let us to see where app is broken and where it is stable, and the best practices to apply. Plus, we'll be able to work on dedicated parts each of us at the same time. It's on the right way!

[aenario]

Could you try to run

./node_modules/.bin/coffee-jshint --options browser,node,undef,unused client/app/**/*.coffee --globals t  > lint.log

Most of them are not your fault, but i see some in account_action_creator, which will crash the client on mailbox manipulation.

103:20: 'account' is not defined.
117:20: 'account' is not defined.
131:20: 'account' is not defined.

[aenario]

REMOVE_ACCOUNT_XXX actions are comented in AppConstants but used.

[aenario]

MAYBELATER

There is quite a few unused require in stores & action_creators (for instance the layout action creator requires but do not use XHRUtils, SocketUtils, MessageStore, AccountActionCreator & MessageActionCreator).

This is a sign that you've done a good job cleaning the mess in the dependencies graph :+1:

[aenario]

router_actioncreator.coffee#L28 Prefer for key, value of params instead of .each

[aenario]

MAYBELATER

router_action_creator.coffee#L46 French comment makes @clochix happy but we try to be welcoming to people who dont have the chance to speak french :-p (will be gone with #FIXME anyway)

[aenario]

components/application.coffee I would move the Router.getLayoutSettings to getStateFrom Stores and className = "" + "..." + to the render method in order to get rid of the whole props part.

[aenario]

panel.coffee this file is not used anymore ( :tada: thanks !) kill it :skull:

[aenario]

getters/routers.coffee You have a lot of if/unless (variable = condition) where you dont use the variable after. isFlags takes one argument, you use it with two

getters in general : Not sure if they should be a class or map of function, will probably depends on how we do memoization.

[aenario]

getters/selection Making it two functions instead of getProps avoid the strange _.extend in message_list

[aenario]

Could you explain your changes in store_watch_mixin ?

I find the new approach harder to read with the mix in getStateFromStores & getInitialState.

[aenario]

missing require AccountStore in NotificationStore

[aenario]

I see you added some logic in XHRUtils, IMHO, it should go back to action_creator.

[aenario]

MAYBELATER

I think we should move all the _current and _selected variables to the routerStore, and get rid of LayoutActionCreator.saveMessage but this can be part of another PR. The current state is already much cleaner !

[frankrousseau]

\o/

[misstick]

I thought that moment would never happen! https://c1.staticflickr.com/5/4025/4488876837_8d3da2423a_z.jpg