[
http://jira.magnolia-cms.com/browse/MGNLUI-187?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jan Haderka reopened MGNLUI-187:
--------------------------------
- FormDialogPresenterImpl
{noformat}
+ /**
+ * This is needed to acces properties from the parent. Currently only
the i18basename.
+ *
+ */
+ Dialog dialog = new Dialog(dialogDefinition);
{noformat}
-- pls use javadoc comment style in javadoc only. Inline comment here is
perfectly appropriate.
- FormPresenterImpl
{noformat}
+ * @param item
+ * @param callback
+ * @return
{noformat}
-- if you are not going to provide info about params or return type don't even
add this to javadoc. It will be generated empty in javadoc anyway.
- CreateItemAction
{noformat}
/**
49 * CreateItemAction.
50 */
51 public class CreateItemAction extends ActionBase<CreateItemActionDefinition>
{
{noformat}
-- while this kind of javadoc will make checkstyle to shut up, it's not enough
for me.
{noformat}
/**
70 * The ItemSubApp only gets a location containing nodePath and View
Type.
71 * When creating a new node, we either create it here and pass the
new path to the subapp or
72 * we pass all needed parameters to the location. This is less
messy, but not optimal.. at all.
73 */
{noformat}
-- again either this is javadoc and should be placed in javadoc or it is an
inline comment and should use inline comment style
{noformat}
public void execute() throws ActionExecutionException {
...
81 } catch (RepositoryException e) {
82 log.error("Could not create child node on parent.", e);
83 }
{noformat}
-- This is sooo wrong! Obviously Action could not be executed because of
RepoException, so you should rethrow it as AEE and not just log in log files
where no one will see it.
- CreateItemActionDefinition
-- Class level javadoc is not correct.
-- Also action def that uses appId, subAppId and NodeType seems to me like
something that will be used around much more often then just to CreateItems so
perhaps name should be more generic
- ItemWorkbenchPresenter
{noformat}
public void onCancel() {
+ // should switch to ViewType.VIEW
}
{noformat}
-- thx for explanation, but is there a ticket for it created already? And
really this is just a "TODO" w/o "TODO" so if you really need to leave it like
this you should mark it as todo (And create ticket in jira)
- config.modules.ui-admincentral.workbenchActionRegistry.xml
{noformat}
+ <sv:node sv:name="MetaData">
+ <sv:property sv:name="jcr:primaryType" sv:type="Name">
+ <sv:value>mgnl:metaData</sv:value>
+ </sv:property>
+ <sv:property sv:name="jcr:createdBy" sv:type="String">
+ <sv:value>admin</sv:value>
+ </sv:property>
+ </sv:node>
+ </sv:node>
{noformat}
-- remove, no metadata should be in new bootstrap files
- SaveContactFormAction
{noformat}
+ // Can't use this anymore, breaks when renaming node,
ContentChangedEvent is still using the old path
+ //generateUniqueNodeNameForContact(node);
{noformat}
-- either this we need to do something about it, in which case it is TODO and
there should be jira ticket or it is not a TODO, we plan to do nothing about it
and it should be removed completely from the code
{noformat}
+ } else {
+ //validation errors are displayed in the UI.
+ }
{noformat}
-- else block just to make inline comment is terrible waste of space and your
and compiler's time.
{noformat}
generateUniqueNodeNameForContact()
{noformat}
-- we have already utilities in core to generate unique names. Why not reuse
them rather then inventing your own code?
- CancelFormActionDefinition
-- class level javadoc explaining why we need empty class implementing empty
interface?
- FormActionFactoryImpl
-- class level javadoc
- SaveFormActionDefinition
- class level javadoc
- this def defines only label and name. I doubt it is relevant only to save so
it should be perhaps renames to something more generic
- ItemWorkbenchPresenter
{noformat}
+ //final FormPresenter formPresenter =
formPresenterFactory.createFormPresenterByName(workbenchDefinition.getFormName());
+ final FormPresenter formPresenter =
formPresenterFactory.createFormPresenterByDefinition(workbenchDefinition.getFormDefinition());
{noformat}
-- Why are you adding commented lines of code?
- ui-admincentral.xml
{noformat}
+ <component>
+ <type>info.magnolia.ui.vaadin.form.FormView</type>
+
<implementation>info.magnolia.ui.vaadin.form.FormViewImpl</implementation>
+ </component>
{noformat}
-- why is ui-vaadin component defined in ui-admincentral and not in ui-vaadin?
- ContactsModule
-- some field labels seems to be capitalized (Office Fax Nr) some not (Office
phone) ... I don't really care which style you choose (pbly Antti or Andreas
should say), but they need to be all written in same style
-- button labels seem to be completely w/o capitalisation and all lower case
- SaveContactFormAction, SaveContactFormActionDefinition, FormActionBuilder,
FormBuilder, FormConfig
-- class level javadoc, btw 2nd class defines nothing ... why is it needed at
all?
- in case i missed some class, all that have just name of the class w/ dot at
the end as a javadoc need to be updated.
> itemsubapp: actions for saving node from forms
> ----------------------------------------------
>
> Key: MGNLUI-187
> URL: http://jira.magnolia-cms.com/browse/MGNLUI-187
> Project: Magnolia UI
> Issue Type: Sub-task
> Security Level: Public
> Affects Versions: 5.0 Alpha1 s008
> Reporter: Espen Jervidalo
> Assignee: Espen Jervidalo
> Fix For: 5.0 Alpha1 s009
>
>
> see referenced git commits, and also:
> https://git.magnolia-cms.com/gitweb/?p=magnolia_ui.git;a=commit;h=9bbc91cbf63da076e776ac17351a72d578230696
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://jira.magnolia-cms.com/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
----------------------------------------------------------------
For list details, see: http://www.magnolia-cms.com/community/mailing-lists.html
Alternatively, use our forums: http://forum.magnolia-cms.com/
To unsubscribe, E-mail to: <[email protected]>
----------------------------------------------------------------
