Bug 1261488 - prototype window-stapled sidebar using different XUL browsers to display TOC + sidebar or just sidebar#329
Bug 1261488 - prototype window-stapled sidebar using different XUL browsers to display TOC + sidebar or just sidebar#329mancas wants to merge 25 commits intomozilla:masterfrom
Conversation
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
12 similar comments
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
|
Autolander could not find a bug number in your pull request title. All pull requests should be in the format of: Bug [number] - [description]. |
| <head> | ||
| <meta charset="utf-8"> | ||
| <base href="chrome://loop/content"> | ||
| <link rel="stylesheet" type="text/css" href="shared/css/reset.css"> |
There was a problem hiding this comment.
Please add an XXX to review all these inclusions once the dust settles a bit. I don't want us to include unnecessary css/js files.
|
There's a lot of comments here. I don't think this should land without my the comments about de-forking / moving code into more appropriate files being addressed. Without those, it makes it very hard to reason about what code is used where, and the expectations and what actually changed. I think it'll also make it a lot harder to track any changes on master, especially as git would do some of these for us. The next thing I think that must be worked on is the WebRTC / ToC separation. It feels too intermingled here and that's going to make it hard for more than one person to work on this at a time. Hence I'm hesitant to agree we should land this now - however, splitting the WebRTC/ToC is likely to be a big chunk in itself, so as long as that's the next chunk I think that's probably OK, as if necessary it'll still be easy to diff back to master to see the overall differences. On the commits front, I think we should probably squash at least some of them, or give them proper comments, things like "sidebar" isn't useful at all (and, of course, they should get the bug number). |
| * @return {Boolean} | ||
| */ | ||
| _roomIsActive: function() { | ||
| return this.state.roomState === ROOM_STATES.JOINED || |
There was a problem hiding this comment.
It feels like we've got too much webrtc related stuff intermingled in this component for what should be a simple layout component. I think we need to reconsider the component levels, and either have another level in-between or make the sub-components more able to stand by themselves.
There was a problem hiding this comment.
This component was forked from StandaloneRoomView so using the original we don't need to add more logic to it but I'd like to do your suggestion in a next step.
4 participants
No description provided.