Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Start to factor out session-loading magic #397

Merged
merged 2 commits into from
Aug 10, 2016
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Aug 9, 2016

Take some of the magic out of MatrixChat.componentDidMount() into a new component.

There will be more of this; there is some in vector/index.js, and more in MatrixClientPeg, but I wanted to start here :)

Also delete the MatrixChat test. It wasn't really doing much, is broken by the change, and I am replacing it with (better) app-level tests in the vector project.

Take some of the magic out of MatrixChat.componentDidMount() into a new
component.

Also delete the MatrixChat test. It wasn't really doing much, is broken by the
change, and I am replacing it with (better) app-level tests in the vector
project.
@dbkr
Copy link
Member

dbkr commented Aug 10, 2016

Feels a little odd this being in a react component when it's UI is just a spinner - is there a reason it's not just a class with the _loadSession() function?

Also, if it is a react component, is there a reason it's loaded directly, bypassing the skin?

@dbkr dbkr assigned richvdh and unassigned dbkr Aug 10, 2016
@richvdh
Copy link
Member Author

richvdh commented Aug 10, 2016

Feels a little odd this being in a react component when it's UI is just a spinner - is there a reason it's not just a class with the _loadSession() function?

Er, no. good point. will update it.

@richvdh richvdh assigned dbkr and unassigned richvdh Aug 10, 2016
There's no point in it being a React component.
@richvdh richvdh force-pushed the rav/factor_out_sessionloader branch from bd1d46e to 26c7c9e Compare August 10, 2016 10:36
@dbkr dbkr merged commit e0f7197 into develop Aug 10, 2016
@richvdh richvdh deleted the rav/factor_out_sessionloader branch August 10, 2016 10:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants