-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3960/#review6572
-----------------------------------------------------------
Thanks, great work!
Basically the patch is good. But I think it would be better to use regular
JUnit instead of GWT one. The issue with JSON parsing can be solved by
encapsulating the parsing logic into separate class that can be injected into
GadgetInfoProviderImpl - for example
public interface GadgetInfoParser {
public List<GadgetInfo> parseGadgetInfoJson(String json);
}
Then we can have two implementations of this interface.
For use in application:
GadgetInfoProviderImpl gadgetInfoProvider = new GadgetInfoProviderImpl(new
GwtGadgetInfoParser());
For JUnit we can just inject a mock implementation to return the gadget infos
or use some server side implementation of JSON parser.
- Yuri
On 2012-03-24 13:30:50, rocklund wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3960/
> -----------------------------------------------------------
>
> (Updated 2012-03-24 13:30:50)
>
>
> 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
> -----
>
> jsongadgets.json PRE-CREATION
> src/org/waveprotocol/box/server/ServerMain.java 13a2d55
> src/org/waveprotocol/box/server/persistence/file/FileUtils.java 538e6e3
> src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java PRE-CREATION
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/EditToolbar.java
> 3a470e0
>
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
> PRE-CREATION
>
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.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/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
>
>
