[
https://issues.apache.org/jira/browse/WAVE-319?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233649#comment-13233649
]
[email protected] commented on WAVE-319:
----------------------------------------------------
-----------------------------------------------------------
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:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3960/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-17 20:25:34)
bq.
bq.
bq. Review request for wave.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. * Added more gadgets to the gadget list
bq. * Made the gadget list scrollable and filterable through a text box and a
drop down box for categories.
bq. * The filtering looks at both the name of the gadget and its description.
Author could also be added as a searchable property (?)
bq. * Filter the result directly in the scrollable gadget list
bq. * Marking the top filtered search as selected with gray background and
made it possible to choose that gadget by pressing enter
bq. * 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
bq.
bq.
bq. This addresses bug wave-319.
bq. https://issues.apache.org/jira/browse/wave-319
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.ui.xml
cc6b73e
bq. src/org/waveprotocol/wave/client/widget/popup/CenterPopupPositioner.java
555f32e
bq. src/org/waveprotocol/wave/client/widget/popup/PositionUtil.java 5e3bab4
bq.
test/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderTest.java
PRE-CREATION
bq.
src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoWidget.java
97611b4
bq.
src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoWidget.ui.xml
c8b7a81
bq.
src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetSelectorWidget.java
ccbcdae
bq.
src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProviderImpl.java
PRE-CREATION
bq. src/org/waveprotocol/box/server/ServerMain.java 13a2d55
bq. src/org/waveprotocol/box/server/persistence/file/FileUtils.java 538e6e3
bq. src/org/waveprotocol/box/server/rpc/GadgetProviderServlet.java
PRE-CREATION
bq. src/org/waveprotocol/box/webclient/client/WebClient.java c87b7cc
bq.
src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/gadget/GadgetInfoProvider.java
PRE-CREATION
bq. jsongadgets.json PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/3960/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested locally.
bq. Successfully run all junit test (Except
PerUserWaveViewSubscriberTest::testGetPerUserWaveView and
WaveServerTest::testWaveletNotification that fails even before this patch)
bq. Tested all included gadgets
bq.
bq.
bq. Thanks,
bq.
bq. rocklund
bq.
bq.
> Improve the "Add gadget" popup
> -------------------------------
>
> Key: WAVE-319
> URL: https://issues.apache.org/jira/browse/WAVE-319
> Project: Wave
> Issue Type: Improvement
> Components: Web Client
> Reporter: Yuri Zelikov
> Assignee: Olof
> Priority: Minor
> Labels: StarterProject
>
> 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
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
