Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML-705: HTML Form Entry tag for creating a single condition #17

Closed
wants to merge 7 commits into from

Conversation

samuelmale
Copy link
Member

No description provided.

@mks-d mks-d changed the title Ra 1608 RA-1608: WIP Jul 12, 2019
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess will need to update the .gitignore file

api-2.2/.project Outdated
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same case here

Copy link
Member

@mks-d mks-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to update the .gitignore to ensure that the new api-2.2 folder is subject to the usual rules.

And of course, tests!

@mks-d mks-d changed the title RA-1608: WIP HTML-705 HTML Form Entry tag for creating a single condition Jul 16, 2019
@mks-d mks-d self-requested a review July 16, 2019 08:25
StringBuilder ret = new StringBuilder();
ret.append("<h5>");
ret.append(mss.getMessage("coreapps.conditionui.addNewCondition"));
ret.append("</h5>");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want any title. Labels will be added when the tag is being used on the form.

return ret.toString();
}

public String createConditionNameWidget(FormEntryContext context) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you changed this signature, but why has this become public then?

I would rather see a protected method named htmlForConditionSearchWidget(FormEntryContext) so that it reads like this:

ret.append(htmlForConditionSearchWidget(context));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok man, that naming sounds just perfect.

I see that you changed this signature, but why has this become public then?

Actually that was a mistake, your right anyway

return conditionStatusWidget.generateHtml(context) + sb.toString();
}

private String createConditionDateWidgets(FormEntryContext context) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather see a protected method named htmlForConditionDateWidgets(FormEntryContext).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we have it private ?

return ret.toString();
}

private String createConditionStatusWidgets(FormEntryContext context) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather see a protected method named htmlForConditionStatusWidgets(FormEntryContext).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we have it private ?

private DateWidget onsetDate;

@Before
public void setup() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setup enforces too many things IMO.

Let's discuss this.

@mks-d mks-d changed the title HTML-705 HTML Form Entry tag for creating a single condition HTML-705: HTML Form Entry tag for creating a single condition Jul 16, 2019
@samuelmale samuelmale closed this Jul 16, 2019
@samuelmale samuelmale reopened this Jul 16, 2019
@samuelmale
Copy link
Member Author

New draft PR : #18

@samuelmale samuelmale closed this Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants