----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/#review5635 -----------------------------------------------------------
Great progress! Thanks for doing this. Some comments below. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java <https://reviews.apache.org/r/3960/#comment12268> Can we just extract this code into a method with corresponding name instead of adding comment? Also for code/comments below. It always better to write self documenting code. If there's some code that requires a comment - usually it means that this snippet can be extracted into a method of it's own. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java <https://reviews.apache.org/r/3960/#comment12269> No need for adding too much comments if the code is self descriptive like in this case. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/JsonGadgetInfo.java <https://reviews.apache.org/r/3960/#comment12270> How about to make this list available on the server side? We can add a servlet that will read the json file and server it to the client. This way adding a new gadget definition will be as simple as editing json formatted file. test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java <https://reviews.apache.org/r/3960/#comment12271> Empty line. - Yuri On 2012-03-04 22:32:02, rocklund wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3960/ > ----------------------------------------------------------- > > (Updated 2012-03-04 22:32:02) > > > Review request for wave. > > > Summary > ------- > > * Added more gadgets to the gadget list > * Made the gadget list scrollable and filterable through a text box and a > drop down box for categories. > * The filtering looks at both the name of the gadget and its description. > Author could also be added as a searchable property (?) > * Filter the result directly in the scrollable gadget list > * Marking the top filtered search as selected with gray background and made > it possible to choose that gadget by pressing enter > * Change the default focus to the filter box to allow the user to quickly > select a gadget from the list by filter it out and pressing enter > > > This addresses bug wave-319. > https://issues.apache.org/jira/browse/wave-319 > > > Diffs > ----- > > > src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java > PRE-CREATION > > src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoWidget.java > 97611b4 > > src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoWidget.ui.xml > c8b7a81 > > src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java > ccbcdae > > src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.ui.xml > cc6b73e > > src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/JsonGadgetInfo.java > PRE-CREATION > src/org/waveprotocol/wave/client/widget/popup/CenterPopupPositioner.java > 555f32e > src/org/waveprotocol/wave/client/widget/popup/PositionUtil.java 5e3bab4 > > test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/3960/diff > > > Testing > ------- > > Tested locally. > Successfully run all junit test (Except > PerUserWaveViewSubscriberTest::testGetPerUserWaveView and > WaveServerTest::testWaveletNotification that fails even before this patch) > Tested all included gadgets > > > Thanks, > > rocklund > >
