Wave
  1. Wave
  2. WAVE-319

Improve the "Add gadget" popup

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Web Client
    • Labels:

      Description

      Improve the "Add gadget" popup. Currently it contains only a handful of gadgets. Add more gadget definitions, make the panel scrollable, add ability to filter the gadgets list and or add suggestion box - http://gwt.google.com/samples/Showcase/Showcase.html#!CwSuggestBox
      The gadget definitions can be taken from http://goo.gl/nEem2

        Activity

        Hide
        Jeremy Naegel added a comment -

        shouldn't this issue be closed?
        Congrats on the great work on this btw!

        Show
        Jeremy Naegel added a comment - shouldn't this issue be closed? Congrats on the great work on this btw!
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/#review6766
        -----------------------------------------------------------

        Ship it!

        LGTM.
        Committed as r1310687

        • Yuri

        On 2012-04-01 17:49:43, rocklund wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3960/

        -----------------------------------------------------------

        (Updated 2012-04-01 17:49:43)

        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/GadgetSelectorWidget.ui.xml cc6b73e

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GwtGadgetInfoParser.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

        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/GadgetInfoWidget.java 97611b4

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java PRE-CREATION

        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/GadgetInfoParser.java PRE-CREATION

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java PRE-CREATION

        src/org/waveprotocol/box/server/ServerMain.java 13a2d55

        jsongadgets.json 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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/#review6766 ----------------------------------------------------------- Ship it! LGTM. Committed as r1310687 Yuri On 2012-04-01 17:49:43, rocklund wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-04-01 17:49:43) 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/GadgetSelectorWidget.ui.xml cc6b73e src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GwtGadgetInfoParser.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 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/GadgetInfoWidget.java 97611b4 src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java PRE-CREATION 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/GadgetInfoParser.java PRE-CREATION src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java PRE-CREATION src/org/waveprotocol/box/server/ServerMain.java 13a2d55 jsongadgets.json 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        (Updated 2012-04-01 17:49:43.404931)

        Review request for wave.

        Changes
        -------

        Sure, updated to use Mockito instead

        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 (updated)


        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.ui.xml cc6b73e
        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GwtGadgetInfoParser.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
        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/GadgetInfoWidget.java 97611b4
        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java PRE-CREATION
        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/GadgetInfoParser.java PRE-CREATION
        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java PRE-CREATION
        src/org/waveprotocol/box/server/ServerMain.java 13a2d55
        jsongadgets.json 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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-04-01 17:49:43.404931) Review request for wave. Changes ------- Sure, updated to use Mockito instead 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 (updated) src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.ui.xml cc6b73e src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GwtGadgetInfoParser.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 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/GadgetInfoWidget.java 97611b4 src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java PRE-CREATION 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/GadgetInfoParser.java PRE-CREATION src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java PRE-CREATION src/org/waveprotocol/box/server/ServerMain.java 13a2d55 jsongadgets.json 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/#review6607
        -----------------------------------------------------------

        test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java
        <https://reviews.apache.org/r/3960/#comment14276>

        Can we instead use mockito to mock the GadgetInfoParser?
        Something like this:

        List<GadgetInfo> gadgetList = new ArrayList<GadgetInfo>();

        gadgetList.add(gadget1);
        gadgetList.add(gadget2);
        gadgetList.add(gadget3)
        GadgetInfoParser parser = mock(GadgetInfoParser.class);
        when(parser.parseGadgetInfoJson(anyString()).thenReturn(gadgetList);

        • Yuri

        On 2012-04-01 16:43:42, rocklund wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3960/

        -----------------------------------------------------------

        (Updated 2012-04-01 16:43:42)

        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/GadgetInfoParser.java PRE-CREATION

        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/wavepanel/impl/toolbar/gadget/GwtGadgetInfoParser.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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/#review6607 ----------------------------------------------------------- test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java < https://reviews.apache.org/r/3960/#comment14276 > Can we instead use mockito to mock the GadgetInfoParser? Something like this: List<GadgetInfo> gadgetList = new ArrayList<GadgetInfo>(); gadgetList.add(gadget1); gadgetList.add(gadget2); gadgetList.add(gadget3) GadgetInfoParser parser = mock(GadgetInfoParser.class); when(parser.parseGadgetInfoJson(anyString()).thenReturn(gadgetList); Yuri On 2012-04-01 16:43:42, rocklund wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-04-01 16:43:42) 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/GadgetInfoParser.java PRE-CREATION 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/wavepanel/impl/toolbar/gadget/GwtGadgetInfoParser.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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        (Updated 2012-04-01 16:43:42.518573)

        Review request for wave.

        Changes
        -------

        Thanks again for your feedback Yuri.

        Updated according to your recommendation and is now running normal JUnit test.

        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 (updated)


        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/GadgetInfoParser.java PRE-CREATION
        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/wavepanel/impl/toolbar/gadget/GwtGadgetInfoParser.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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-04-01 16:43:42.518573) Review request for wave. Changes ------- Thanks again for your feedback Yuri. Updated according to your recommendation and is now running normal JUnit test. 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 (updated) 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/GadgetInfoParser.java PRE-CREATION 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/wavepanel/impl/toolbar/gadget/GwtGadgetInfoParser.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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        (Updated 2012-03-24 13:30:50.312549)

        Review request for wave.

        Changes
        -------

        Updated according to the comments. Thanks.

        The reason why I have GWTTestCase instead of normal TestCase is that it is needed for the JSON parsing to work.

        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 (updated)


        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-03-24 13:30:50.312549) Review request for wave. Changes ------- Updated according to the comments. Thanks. The reason why I have GWTTestCase instead of normal TestCase is that it is needed for the JSON parsing to work. 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 (updated) 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/#review6128
        -----------------------------------------------------------

        Thanks for the effort - the patch is almost done!
        Just some more comments.

        src/org/waveprotocol/box/server/persistence/file/FileUtils.java
        <https://reviews.apache.org/r/3960/#comment13165>

        Open -> Opens

        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
        <https://reviews.apache.org/r/3960/#comment13166>

        A servlet -> the servlet.

        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
        <https://reviews.apache.org/r/3960/#comment13167>

        LOGGER -> LOG
        The standard name for logger in WIAB is LOG.

        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
        <https://reviews.apache.org/r/3960/#comment13168>

        Please also include the actual exception in the log.

        -> LOG.log(Level.WARNING, Error while loading gadgets json", e);

        src/org/waveprotocol/box/webclient/client/WebClient.java
        <https://reviews.apache.org/r/3960/#comment13169>

        After thinking a bit more about the injection, it seems like the more natural place to inject the actual implementation would in EditToolbar.
        Instead of:
        GadgetSelectorWidget selector = new GadgetSelectorWidget();

        -> GadgetSelectorWidget selector = new GadgetSelectorWidget(new GadgetInfoProviderImpl());

        Sorry, for the mess.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment13176>

        Please make an interface. I mean there should be an interface, and a concrete implementation. No need for abstract class here.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java
        <https://reviews.apache.org/r/3960/#comment13170>

        Why trace? Probably warning or error is more appropriate.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java
        <https://reviews.apache.org/r/3960/#comment13171>

        Same here.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java
        <https://reviews.apache.org/r/3960/#comment13173>

        We should inject the instance in the constructor.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java
        <https://reviews.apache.org/r/3960/#comment13174>

        Line too long?

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java
        <https://reviews.apache.org/r/3960/#comment13175>

        Note -> TODO

        test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java
        <https://reviews.apache.org/r/3960/#comment13178>

        Are you sure we really need GWTTestCase here?

        • Yuri

        On 2012-03-17 20:25:34, rocklund wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3960/

        -----------------------------------------------------------

        (Updated 2012-03-17 20:25:34)

        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/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

        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/GadgetInfoProviderImpl.java 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/box/webclient/client/WebClient.java c87b7cc

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java PRE-CREATION

        jsongadgets.json 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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/#review6128 ----------------------------------------------------------- Thanks for the effort - the patch is almost done! Just some more comments. src/org/waveprotocol/box/server/persistence/file/FileUtils.java < https://reviews.apache.org/r/3960/#comment13165 > Open -> Opens src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java < https://reviews.apache.org/r/3960/#comment13166 > A servlet -> the servlet. src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java < https://reviews.apache.org/r/3960/#comment13167 > LOGGER -> LOG The standard name for logger in WIAB is LOG. src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java < https://reviews.apache.org/r/3960/#comment13168 > Please also include the actual exception in the log. -> LOG.log(Level.WARNING, Error while loading gadgets json", e); src/org/waveprotocol/box/webclient/client/WebClient.java < https://reviews.apache.org/r/3960/#comment13169 > After thinking a bit more about the injection, it seems like the more natural place to inject the actual implementation would in EditToolbar. Instead of: GadgetSelectorWidget selector = new GadgetSelectorWidget(); -> GadgetSelectorWidget selector = new GadgetSelectorWidget(new GadgetInfoProviderImpl()); Sorry, for the mess. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment13176 > Please make an interface. I mean there should be an interface, and a concrete implementation. No need for abstract class here. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java < https://reviews.apache.org/r/3960/#comment13170 > Why trace? Probably warning or error is more appropriate. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java < https://reviews.apache.org/r/3960/#comment13171 > Same here. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java < https://reviews.apache.org/r/3960/#comment13173 > We should inject the instance in the constructor. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java < https://reviews.apache.org/r/3960/#comment13174 > Line too long? src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java < https://reviews.apache.org/r/3960/#comment13175 > Note -> TODO test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java < https://reviews.apache.org/r/3960/#comment13178 > Are you sure we really need GWTTestCase here? Yuri On 2012-03-17 20:25:34, rocklund wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-03-17 20:25:34) 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/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 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/GadgetInfoProviderImpl.java 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/box/webclient/client/WebClient.java c87b7cc src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java PRE-CREATION jsongadgets.json 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        (Updated 2012-03-17 20:25:34.239494)

        Review request for wave.

        Changes
        -------

        Please review patch. Thanks!

        • Created GadgetInfoProvider interface and GadgetInfoProviderImpl that implements it
        • Created manual injection of the GadgetInfoProvider in WebClient.java - I think this might not be exactly as you meant @Yuri, maybe you can point me in the right direction if you don't think this solution is good? I could not find any similar example in the current web client source code.
        • Added proper error handling and error logging
        • Updated according to the comments

        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 (updated)


        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
        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/GadgetInfoProviderImpl.java 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/box/webclient/client/WebClient.java c87b7cc
        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java PRE-CREATION
        jsongadgets.json 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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-03-17 20:25:34.239494) Review request for wave. Changes ------- Please review patch. Thanks! Created GadgetInfoProvider interface and GadgetInfoProviderImpl that implements it Created manual injection of the GadgetInfoProvider in WebClient.java - I think this might not be exactly as you meant @Yuri, maybe you can point me in the right direction if you don't think this solution is good? I could not find any similar example in the current web client source code. Added proper error handling and error logging Updated according to the comments 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 (updated) 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 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/GadgetInfoProviderImpl.java 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/box/webclient/client/WebClient.java c87b7cc src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java PRE-CREATION jsongadgets.json 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/#review5884
        -----------------------------------------------------------

        You did really great work!
        Just some comments below.
        Regarding your questions:
        1. I think it's OK to put in the root folder.
        2. Probably it would be better to cache the json in the memory. For example we can use ComputingMap with expireOnWrite() option set to 5 minutes.

        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
        <https://reviews.apache.org/r/3960/#comment12825>

        Most server side code uses the standard Java Logger for logging.

        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
        <https://reviews.apache.org/r/3960/#comment12826>

        Maybe we need to extract this code into a method and add it to the FileUtils class.

        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
        <https://reviews.apache.org/r/3960/#comment12827>

        Probably it would be better to use StringBuilder for line variable instead of String.

        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
        <https://reviews.apache.org/r/3960/#comment12828>

        It seems like Exception is too general, can we replace it with just IOException?

        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
        <https://reviews.apache.org/r/3960/#comment12829>

        Please wrap "+" with spaces

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment12831>

        Can we use more meaningful name for the category type instead of "value"? How about "type"?

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment12830>

        Maybe just use equalsIgnoreCase() here?

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment12832>

        static public -> public static

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment12833>

        Can we change the access modifier for the GadgetInfo from public to private? If we want to access the fields - we would better provide getters.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment12835>

        Please terminate the comments with full stop.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment12836>

        Also here and below

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment12837>

        Can we use more meaningful name instead of "myFilter"? Or maybe just use equalsIgnoreCase()? Also, please access the gadgetInfo fields via corresponding getters.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
        <https://reviews.apache.org/r/3960/#comment12838>

        Please move the constructor up.
        It should be declared just below the fields

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java
        <https://reviews.apache.org/r/3960/#comment12839>

        These new variables have default access modifier, can we change it to private?

        src/org/waveprotocol/wave/client/widget/popup/CenterPopupPositioner.java
        <https://reviews.apache.org/r/3960/#comment12840>

        I had long discussion about the value of the offset with David Hearnden and his opinion was that the most natural value for offset is 0. I ll try to find it and send you the link.

        src/org/waveprotocol/wave/client/widget/popup/PositionUtil.java
        <https://reviews.apache.org/r/3960/#comment12841>

        Restrict -> Restricts
        Also, please terminate the comments in javadoc with full stop.

        test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java
        <https://reviews.apache.org/r/3960/#comment12842>

        Please add @author

        test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java
        <https://reviews.apache.org/r/3960/#comment12843>

        Do we really need GWTTestCase here? Can we do with just regular JUnit test?

        • Yuri

        On 2012-03-11 17:31:31, rocklund wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3960/

        -----------------------------------------------------------

        (Updated 2012-03-11 17:31:31)

        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/rpc/GadgetProviderServlet.java PRE-CREATION

        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/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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/#review5884 ----------------------------------------------------------- You did really great work! Just some comments below. Regarding your questions: 1. I think it's OK to put in the root folder. 2. Probably it would be better to cache the json in the memory. For example we can use ComputingMap with expireOnWrite() option set to 5 minutes. src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java < https://reviews.apache.org/r/3960/#comment12825 > Most server side code uses the standard Java Logger for logging. src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java < https://reviews.apache.org/r/3960/#comment12826 > Maybe we need to extract this code into a method and add it to the FileUtils class. src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java < https://reviews.apache.org/r/3960/#comment12827 > Probably it would be better to use StringBuilder for line variable instead of String. src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java < https://reviews.apache.org/r/3960/#comment12828 > It seems like Exception is too general, can we replace it with just IOException? src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java < https://reviews.apache.org/r/3960/#comment12829 > Please wrap "+" with spaces src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment12831 > Can we use more meaningful name for the category type instead of "value"? How about "type"? src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment12830 > Maybe just use equalsIgnoreCase() here? src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment12832 > static public -> public static src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment12833 > Can we change the access modifier for the GadgetInfo from public to private? If we want to access the fields - we would better provide getters. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment12835 > Please terminate the comments with full stop. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment12836 > Also here and below src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment12837 > Can we use more meaningful name instead of "myFilter"? Or maybe just use equalsIgnoreCase()? Also, please access the gadgetInfo fields via corresponding getters. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java < https://reviews.apache.org/r/3960/#comment12838 > Please move the constructor up. It should be declared just below the fields src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java < https://reviews.apache.org/r/3960/#comment12839 > These new variables have default access modifier, can we change it to private? src/org/waveprotocol/wave/client/widget/popup/CenterPopupPositioner.java < https://reviews.apache.org/r/3960/#comment12840 > I had long discussion about the value of the offset with David Hearnden and his opinion was that the most natural value for offset is 0. I ll try to find it and send you the link. src/org/waveprotocol/wave/client/widget/popup/PositionUtil.java < https://reviews.apache.org/r/3960/#comment12841 > Restrict -> Restricts Also, please terminate the comments in javadoc with full stop. test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java < https://reviews.apache.org/r/3960/#comment12842 > Please add @author test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java < https://reviews.apache.org/r/3960/#comment12843 > Do we really need GWTTestCase here? Can we do with just regular JUnit test? Yuri On 2012-03-11 17:31:31, rocklund wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-03-11 17:31:31) 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/rpc/GadgetProviderServlet.java PRE-CREATION 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/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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        (Updated 2012-03-11 17:31:31.929113)

        Review request for wave.

        Changes
        -------

        Thanks for your nice feedback Juri!

        I updated according to your comments. Still have two things to do:

        • Update GadgetInfoProvider to interface as you suggested
        • Add proper error handling

        Then I wondering if I am doing the correct file handling of the json file?

        • Should I create some kind of FileStore for the file and place it in a sub-folder or is it ok to have it in the server root?
        • Should the GadgetProviderServlet save the content of the file into a memory variable (String) for later use or should it read the content from the file every time?

        Thanks.

        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 (updated)


        jsongadgets.json PRE-CREATION
        src/org/waveprotocol/box/server/ServerMain.java 13a2d55
        src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java PRE-CREATION
        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/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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-03-11 17:31:31.929113) Review request for wave. Changes ------- Thanks for your nice feedback Juri! I updated according to your comments. Still have two things to do: Update GadgetInfoProvider to interface as you suggested Add proper error handling Then I wondering if I am doing the correct file handling of the json file? Should I create some kind of FileStore for the file and place it in a sub-folder or is it ok to have it in the server root? Should the GadgetProviderServlet save the content of the file into a memory variable (String) for later use or should it read the content from the file every time? Thanks. 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 (updated) jsongadgets.json PRE-CREATION src/org/waveprotocol/box/server/ServerMain.java 13a2d55 src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java PRE-CREATION 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/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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/#review5820
        -----------------------------------------------------------

        I checked the patch - it's really makes a huge difference got someone working with gadgets. Great job.
        Basically it's almost ready. Just few more comments.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java
        <https://reviews.apache.org/r/3960/#comment12718>

        Can we make an interface for GadgetInfoProvider? So that an implementation instance would be created in WebClient and injected into this class? So way we allow an easy way to customize the implementation of GadgetProvider.

        src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java
        <https://reviews.apache.org/r/3960/#comment12719>

        Can we use here just the List interface instead of concrete implementation?

        src/org/waveprotocol/wave/client/widget/popup/PositionUtil.java
        <https://reviews.apache.org/r/3960/#comment12717>

        Please add spaces for "*" and "-"

        • 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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/#review5820 ----------------------------------------------------------- I checked the patch - it's really makes a huge difference got someone working with gadgets. Great job. Basically it's almost ready. Just few more comments. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java < https://reviews.apache.org/r/3960/#comment12718 > Can we make an interface for GadgetInfoProvider? So that an implementation instance would be created in WebClient and injected into this class? So way we allow an easy way to customize the implementation of GadgetProvider. src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java < https://reviews.apache.org/r/3960/#comment12719 > Can we use here just the List interface instead of concrete implementation? src/org/waveprotocol/wave/client/widget/popup/PositionUtil.java < https://reviews.apache.org/r/3960/#comment12717 > Please add spaces for "*" and "-" 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        (Updated 2012-03-04 22:32:02.228552)

        Review request for wave.

        Changes
        -------

        Patch done for review! Thanks!

        Changed in recent update:

        • Added JUnit tests for the GadgetInfoProvider class
        • Changed the initialization of the GadgetInfoProvider to take a json string as input parameter
        • Removed non-working gadgets

        Summary (updated)
        -------

        • 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 (updated)


        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 (updated)
        -------

        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-03-04 22:32:02.228552) Review request for wave. Changes ------- Patch done for review! Thanks! Changed in recent update: Added JUnit tests for the GadgetInfoProvider class Changed the initialization of the GadgetInfoProvider to take a json string as input parameter Removed non-working gadgets Summary (updated) ------- 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 (updated) 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 (updated) ------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        (Updated 2012-02-26 17:54:26.007681)

        Review request for wave.

        Changes
        -------

        Updated the format of the gadget infos to a JSON string. However, I realized that it is not possible to open any files for reading on the client side so I will keep it as a static java class for now.

        The only thing remaining now is unit testing of the GadgetInfoProvider. If anyone have any comments/feedback of the current code, please feel free to send in an review.

        Thanks

        Summary (updated)
        -------

        • 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

        Note: The code is Not ready yet. Adding the patch for you to review the new functionality. I am willing to reconsider and of my implementation choices above if there is better alternatives. This is some things I plan to do with the code before I consider it to be ready:

        • Write unit tests for the GadgetInfoProvider class.

        This addresses bug wave-319.
        https://issues.apache.org/jira/browse/wave-319

        Diffs (updated)


        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

        Diff: https://reviews.apache.org/r/3960/diff

        Testing
        -------

        Tested locally. Have not tested all the added gadgets yet though.

        Thanks,

        rocklund

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-02-26 17:54:26.007681) Review request for wave. Changes ------- Updated the format of the gadget infos to a JSON string. However, I realized that it is not possible to open any files for reading on the client side so I will keep it as a static java class for now. The only thing remaining now is unit testing of the GadgetInfoProvider. If anyone have any comments/feedback of the current code, please feel free to send in an review. Thanks Summary (updated) ------- 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 Note: The code is Not ready yet. Adding the patch for you to review the new functionality. I am willing to reconsider and of my implementation choices above if there is better alternatives. This is some things I plan to do with the code before I consider it to be ready: Write unit tests for the GadgetInfoProvider class. This addresses bug wave-319. https://issues.apache.org/jira/browse/wave-319 Diffs (updated) 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 Diff: https://reviews.apache.org/r/3960/diff Testing ------- Tested locally. Have not tested all the added gadgets yet though. Thanks, rocklund
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        (Updated 2012-02-23 23:30:04.244231)

        Review request for wave.

        Changes
        -------

        Getting closer to a real implementation now. The two major points to get done before done is unit testing and JSON loading of gadget info.

        This is updated in this version:

        • Created a new utility function in PositionUtil for calculating the maximum height for a popup to fit the screen and updated CenterPopupPositioner to use this new function
        • Updated the GadgetInfoProvider to handle filtering
        • Updated the GadgetSelectorWidget to use the filtering in the GadgetInfoProvider
        • Polished the UI of the popup to look as it is suppose to
        • Listening to enter event on the gadget url TextBox as well

        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

        Note: The code is Not ready yet. Adding the patch for you to review the new functionality. I am willing to reconsider and of my implementation choices above if there is better alternatives. This is some things I plan to do with the code before I consider it to be ready:

        • Rewrite GadgetInfoProvider to read from a JSON file and preferably handle filtering
        • Write unit tests for the GadgetInfoProvider class.
        • Rewrite the GadgetInfo and GadgetInfoWidget handling in GadgetSelectorWidget
        • Make the popup more beautifully layouted
        • Clean up line endings, indentations and stuff

        This addresses bug wave-319.
        https://issues.apache.org/jira/browse/wave-319

        Diffs (updated)


        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/widget/popup/CenterPopupPositioner.java 555f32e
        src/org/waveprotocol/wave/client/widget/popup/PositionUtil.java 5e3bab4

        Diff: https://reviews.apache.org/r/3960/diff

        Testing
        -------

        Tested locally. Have not tested all the added gadgets yet though.

        Thanks,

        rocklund

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-02-23 23:30:04.244231) Review request for wave. Changes ------- Getting closer to a real implementation now. The two major points to get done before done is unit testing and JSON loading of gadget info. This is updated in this version: Created a new utility function in PositionUtil for calculating the maximum height for a popup to fit the screen and updated CenterPopupPositioner to use this new function Updated the GadgetInfoProvider to handle filtering Updated the GadgetSelectorWidget to use the filtering in the GadgetInfoProvider Polished the UI of the popup to look as it is suppose to Listening to enter event on the gadget url TextBox as well 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 Note: The code is Not ready yet. Adding the patch for you to review the new functionality. I am willing to reconsider and of my implementation choices above if there is better alternatives. This is some things I plan to do with the code before I consider it to be ready: Rewrite GadgetInfoProvider to read from a JSON file and preferably handle filtering Write unit tests for the GadgetInfoProvider class. Rewrite the GadgetInfo and GadgetInfoWidget handling in GadgetSelectorWidget Make the popup more beautifully layouted Clean up line endings, indentations and stuff This addresses bug wave-319. https://issues.apache.org/jira/browse/wave-319 Diffs (updated) 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/widget/popup/CenterPopupPositioner.java 555f32e src/org/waveprotocol/wave/client/widget/popup/PositionUtil.java 5e3bab4 Diff: https://reviews.apache.org/r/3960/diff Testing ------- Tested locally. Have not tested all the added gadgets yet though. Thanks, rocklund
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/#review5278
        -----------------------------------------------------------

        Checked the patch - great progress!

        • Yuri

        On 2012-02-19 17:08:12, rocklund wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3960/

        -----------------------------------------------------------

        (Updated 2012-02-19 17:08:12)

        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

        Note: The code is Not ready yet. Adding the patch for you to review the new functionality. I am willing to reconsider and of my implementation choices above if there is better alternatives. This is some things I plan to do with the code before I consider it to be ready:

        * Rewrite GadgetInfoProvider to read from a JSON file and preferably handle filtering

        * Write unit tests for the GadgetInfoProvider class.

        * Rewrite the GadgetInfo and GadgetInfoWidget handling in GadgetSelectorWidget

        * Make the popup more beautifully layouted

        * Clean up line endings, indentations and stuff

        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/widget/popup/CenterPopupPositioner.java 555f32e

        Diff: https://reviews.apache.org/r/3960/diff

        Testing

        -------

        Tested locally. Have not tested all the added gadgets yet though.

        Thanks,

        rocklund

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/#review5278 ----------------------------------------------------------- Checked the patch - great progress! Yuri On 2012-02-19 17:08:12, rocklund wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- (Updated 2012-02-19 17:08:12) 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 Note: The code is Not ready yet. Adding the patch for you to review the new functionality. I am willing to reconsider and of my implementation choices above if there is better alternatives. This is some things I plan to do with the code before I consider it to be ready: * Rewrite GadgetInfoProvider to read from a JSON file and preferably handle filtering * Write unit tests for the GadgetInfoProvider class. * Rewrite the GadgetInfo and GadgetInfoWidget handling in GadgetSelectorWidget * Make the popup more beautifully layouted * Clean up line endings, indentations and stuff 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/widget/popup/CenterPopupPositioner.java 555f32e Diff: https://reviews.apache.org/r/3960/diff Testing ------- Tested locally. Have not tested all the added gadgets yet though. Thanks, rocklund
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3960/
        -----------------------------------------------------------

        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

        Note: The code is Not ready yet. Adding the patch for you to review the new functionality. I am willing to reconsider and of my implementation choices above if there is better alternatives. This is some things I plan to do with the code before I consider it to be ready:

        • Rewrite GadgetInfoProvider to read from a JSON file and preferably handle filtering
        • Write unit tests for the GadgetInfoProvider class.
        • Rewrite the GadgetInfo and GadgetInfoWidget handling in GadgetSelectorWidget
        • Make the popup more beautifully layouted
        • Clean up line endings, indentations and stuff

        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/widget/popup/CenterPopupPositioner.java 555f32e

        Diff: https://reviews.apache.org/r/3960/diff

        Testing
        -------

        Tested locally. Have not tested all the added gadgets yet though.

        Thanks,

        rocklund

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3960/ ----------------------------------------------------------- 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 Note: The code is Not ready yet. Adding the patch for you to review the new functionality. I am willing to reconsider and of my implementation choices above if there is better alternatives. This is some things I plan to do with the code before I consider it to be ready: Rewrite GadgetInfoProvider to read from a JSON file and preferably handle filtering Write unit tests for the GadgetInfoProvider class. Rewrite the GadgetInfo and GadgetInfoWidget handling in GadgetSelectorWidget Make the popup more beautifully layouted Clean up line endings, indentations and stuff 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/widget/popup/CenterPopupPositioner.java 555f32e Diff: https://reviews.apache.org/r/3960/diff Testing ------- Tested locally. Have not tested all the added gadgets yet though. Thanks, rocklund

          People

          • Assignee:
            Olof
            Reporter:
            Yuri Zelikov
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development