> On Sept. 30, 2012, 2:09 a.m., Angus Turner wrote: > > > > Angus Turner wrote: > sorry my bad, it said > 'some tests would be great if possible' > > rocklund wrote: > I'm not sure if it is possible to add any testing for this since it is > mostly UI-driven code. I couldn't find any tests on wave opening and wave > creation but maybe I missed it? I welcome any suggestions. > > Angus Turner wrote: > I was more thinking the code that copies the old participants over. If it > can't be done that's fine, just seems like something that should be done.
Ah, yes you are right. I can look into separating that code so that it can be tested. - rocklund ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7353/#review12045 ----------------------------------------------------------- On Sept. 29, 2012, 11:53 a.m., rocklund wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7353/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2012, 11:53 a.m.) > > > Review request for wave. > > > Description > ------- > > I've started to create the functionality me and my friends used a lot in > Google Wave - to be able to create a new wave with the participants from an > open wave. (I could not find a JIRA for this) > > The functionality of the patch is working as intended. There is currently a > few things that I would like some input on: > > UI (See the screenshot) > 1. The icon I use is currently only a placeholder icon. Is anyone able to > create a better icon or any ideas of how it can look and I can try to arrange > one? Or is it better to just display plain text? > 2. Is it OK to add this functionality as an icon/text beside the "Add > participant" button for now or should it be placed in a submenu instead that > can eventually contain more functionality? Maybe at least the button should > be placed to the far right edge of the participant panel? > > Implementation > 3. I currently just added the participant addition code into the install > method of the StageTwoProvider. It feels like it might not be the best spot. > Any feedback on where it could be placed? > 4. In the WaveCreationEvent class I only create one > "CREATE_NEW_WAVE_WITH_PARTICIPANTS"-event where I update the participant > pointer. Maybe it is safer to create a new event for every call? > 5. Any other implementation feedback? > > Thanks. > > > Diffs > ----- > > src/org/waveprotocol/box/webclient/client/StageTwoProvider.java 7a6c8ec > src/org/waveprotocol/box/webclient/client/StagesProvider.java 9d83269 > src/org/waveprotocol/box/webclient/client/WebClient.java 863ae6c > src/org/waveprotocol/box/webclient/client/events/WaveCreationEvent.java > 95c317b > > src/org/waveprotocol/box/webclient/client/events/WaveCreationEventHandler.java > adc57f9 > > src/org/waveprotocol/wave/client/wavepanel/impl/edit/ParticipantController.java > a6eeb53 > src/org/waveprotocol/wave/client/wavepanel/view/View.java c770b36 > src/org/waveprotocol/wave/client/wavepanel/view/dom/DomAsViewProvider.java > 64e7f79 > src/org/waveprotocol/wave/client/wavepanel/view/dom/FullStructure.java > e848c5f > src/org/waveprotocol/wave/client/wavepanel/view/dom/full/Participants.css > 66f1836 > > src/org/waveprotocol/wave/client/wavepanel/view/dom/full/ParticipantsViewBuilder.java > 735ec04 > src/org/waveprotocol/wave/client/wavepanel/view/dom/full/TypeCodes.java > ba8bc7a > > src/org/waveprotocol/wave/client/wavepanel/view/dom/full/new_prts_wbutton.png > PRE-CREATION > > Diff: https://reviews.apache.org/r/7353/diff/ > > > Testing > ------- > > Tested manually on a local server both the implementation and the button > placement/adjustment when adding/removing participant and shrinking the > window size > > > Screenshots > ----------- > > > https://reviews.apache.org/r/7353/s/1/ > > > Thanks, > > rocklund > >
