Hi,
> > + /**
> > + * {...@inheritdoc}
> > + */
> > + public String getSpace()
> > + {
> > + return config.getParameter("space", "Main");
> > + }
> > +
> > + /**
> > + * {...@inheritdoc}
> > + */
> > + public String getWiki()
> > + {
> > + return config.getParameter("wiki", "xwiki");
> > + }
>
> Actually I don't see a good reason for which I made these functions public.
> We
> should make them protected.
>
+1, I'll do it.
>
> > +
> > + /**
> > + * {...@inheritdoc}
> > + */
> > + protected String getFileHelpLabel()
> > + {
> > + return Strings.INSTANCE.importOfficeFileHelpLabel();
> > + }
> > +
> > + /**
> > + * {...@inheritdoc}
> > + */
> > + protected void onAttachmentUploaded(Attachment attach, final
> AsyncCallback<Boolean> async)
> > + {
> > +
> > + String fullPageName = getSpace() + "." + getPage();
>
> you should use the ResourceName class to build wiki names, since it's the
> client
> wiki reference serializer/deserializer.
> > +
> WysiwygService.Singleton.getInstance().officeToXHTML(fullPageName,
> getHTMLCleaningParams(),
>
Actually this code will go away when I introduce the new
officeToXHTML(Attachment....) method.
> + // Display the error and avoid submit operation from
> continuing.
> > + displayError(thrown.getMessage());
> > + async.onSuccess(false);
>
> As a convention, we used the onFailure to signal an error on the server or
> in
> the server-client communication (like a GWT failure in the RPC call), while
> the
> submit async.onSuccess(false) is an "application error", a way in which a
> wizard
> step prevents the submit to be done because, for example, the user didn't
> insert
> valid data in the wizard form and another chance is being given to him.
>
> Now, which are the cases in which the importer service fails (throws
> exception)?
> is it correct to do this conversion here (from a server error / exception
> to a
> "validation" error)? or you should rather call async.onFailure()? this only
> depends on the logic of the service function failure.
>
Thanks. I will look into this and fix it.
> + */
> > + public EnumSet<NavigationDirection> getValidDirections()
> > + {
> > + return EnumSet.of(NavigationDirection.FINISH);
>
> We also return CANCEL here, even if a button for it is not displayed and
> it's
> always possible (through the dialog's close button), but in case we will
> ever
> want to display a button, or want to prevent the cancel in certain steps.
>
Ok, I will fix this.
>
> > + public void init(Object data, AsyncCallback< ?> cb)
> > + {
> > + textArea.setHTML("");
> > + textArea.setFocus(true);
>
> I seriously doubt this works, you should a FocusCommand(), look at its
> comment
> for reasons.
>
Ok.
>
> > + protected Map<String, String> getHTMLCleaningParams()
> > + {
> > + Map<String, String> params = new HashMap<String, String>();
> > + params.put("filterStyles", "strict");
> > + // For Office2007: Office2007 generates an xhtml document (when
> copied) which has attributes and tags of
> > + // several namespaces. But the document itself doesn't contain
> the namespace definitions, which causes
> > + // the HTMLCleaner (the DomSerializer) to fail while performing
> it's operations. As a workaround we
> > + // force HTMLCleaner to avoid parsing of namespace information.
> > + params.put("namespacesAware", Boolean.toString(false));
> > + return params;
> > + }
>
> Is this the same function as in the other class?
>
Yes, I am reluctant to introduce a super class or a utility class to wrap up
this functionality. Not sure if I should do it or not.
> + return requestedStep;
>
> Maybe we could create a super class for this, it's the same everywhere.
>
+1.
> Also,
>
> svn propset svn:keywords "Author Id Revision HeadURL" <new-file-path>
>
> for all new added files.
>
> check
>
> http://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HSubversionconfigurationandlineendings
> for how to configure this for your svn client. Note that, even if correctly
> configured, the settings will not be considered by the eclipse plugin (I
> didn't
> manage to make them work), you'll have to do a command-line svn add for
> that to
> work right.
>
Ok, thanks for the tip.
- Asiri
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
