Refactor RoomHeader, and fix topic updates #252
Conversation
Start cleaning up RoomHeader by factoring out a separate SimpleRoomHeader.
Force an update so that we see the latest state. Fixes element-hq/element-web#1299
SimpleRoomHeader and RoomHeader are now separate things (matrix-org/matrix-react-sdk#252), so update Vector accordingly.
|
|
||
| var domain = MatrixClientPeg.get().getDomain(); | ||
|
|
||
| return ( | ||
| <div className="mx_CreateRoom"> | ||
| <RoomHeader simpleHeader="Create room" /> | ||
| <SimpleRoomHeader> | ||
| Create room |
There was a problem hiding this comment.
Hm, I'm quite unconvinced by passing in the title as the contents of the tag. I'd expect simple room headers to have structure for (optional) avatar and title at the least - at which point i'd expect to be passing them in as properties. Is there a rationale i'm missing?
There was a problem hiding this comment.
Only that it looked like the right thing to do given the structure of SimpleRoomHeader. I agree it doesn't lead to a nice api. I'll change it.
|
lgtm other than minor query over SimpleRoomHeader's API - assuming that you've checked that the editing still works; the EditableText impl (via contentEditable) has been a bit fragile, but should be okay now, in terms of correctly keeping the editable text contents in sync with the state of the parent components. But if you haven't reintroduced the bug where the EditableTexts forget their state whenever you re-render() their parent, lgtm |
... rather than a child
|
It seems to work... |
This is in aid of fixing element-hq/element-web#1299,
which is actually trivial once you can see what's going on.
Suggest looking at the individual commits to review: