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

Already on GitHub? Sign in to your account

[Widgets] "previous page" becomes a target #186

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

maxyu commented Aug 3, 2012

No description provided.

@zhizhangchen zhizhangchen and 1 other commented on an outdated diff Aug 3, 2012

src/js/serialize.js
@@ -61,6 +61,10 @@ var DEBUG = true,
attrName = BWidget.getPropertyHTMLAttribute(node.getType(), propName);
propValue = newValue || node.getProperty(propName);
attrValue = propValue;
+ if (typeof attrName === "function") {
+ attrValue = attrName(propValue)["value"];
+ attrName = attrName(propValue)["name"];
@zhizhangchen

zhizhangchen Aug 3, 2012

Contributor

Would it be simpler to replace the above two lines with the following line:
attrName = attrName(propValue);

@maxyu

maxyu Aug 13, 2012

Contributor

updated

@zhizhangchen zhizhangchen and 1 other commented on an outdated diff Aug 3, 2012

src/js/views/property.js
if(ret.result === false) {
$(this).val(node.getProperty(updated));
+ } else if(type === "Button" &&
+ value === "previous page") {
+ ADM.setProperty(node, "opentargetas", "page");
@zhizhangchen

zhizhangchen Aug 3, 2012

Contributor

Is this necessary?

@maxyu

maxyu Aug 13, 2012

Contributor

Yes. It elimated the needs to "disable" the property in serialize.

@zhizhangchen zhizhangchen and 1 other commented on an outdated diff Aug 3, 2012

src/js/views/property.js
- .addClass('title')
- .appendTo(value);
+ if(type === 'Button' && p === 'opentargetas'
+ && node.getProperty('target') ===
+ 'previous page') {
+ $('<select size="1">')
+ .attr('id', valueId)
+ .addClass('title')
+ .appendTo(value)
+ .attr('disabled', 'disabled');
+ } else {
+ $('<select size="1">')
+ .attr('id', valueId)
+ .addClass('title')
+ .appendTo(value);
+ }
@zhizhangchen

zhizhangchen Aug 3, 2012

Contributor

Please simplify the above lines.

@maxyu

maxyu Aug 13, 2012

Contributor

done

@zhizhangchen zhizhangchen and 2 others commented on an outdated diff Aug 3, 2012

src/js/widgets.js
},
opentargetas : {
type: "string",
displayName: "open target as",
- options: ["default", "page", "dialog"],
- defaultValue: "default",
+ options: ["page", "dialog"],
@zhizhangchen

zhizhangchen Aug 3, 2012

Contributor

Why do you remove "default"?

@grgustaf

grgustaf Aug 3, 2012

Contributor

Yeah, I think it's good the way it was, make it clear that you're preserving the page setting.

@maxyu

maxyu Aug 13, 2012

Contributor

Recovered.

Contributor

zhizhangchen commented Aug 13, 2012

Please squash these fixup commits into previous ones.

@zhizhangchen zhizhangchen commented on an outdated diff Aug 14, 2012

src/js/serialize.js
@@ -61,13 +61,29 @@ var DEBUG = true,
attrName = BWidget.getPropertyHTMLAttribute(node.getType(), propName);
propValue = newValue || node.getProperty(propName);
attrValue = propValue;
+ if (typeof attrName === "function") {
+ attrValue = attrName(propValue)["value"];
@zhizhangchen

zhizhangchen Aug 14, 2012

Contributor

This line is not necessary

@zhizhangchen zhizhangchen commented on an outdated diff Aug 14, 2012

src/js/serialize.js
- attrValue = attrMap.value[propValue];
+ switch(typeof(attrValue)) {
+ case "function":
+ attrValue = attr.value(propValue);
+ break;
+ case "object":
+ attrValue = attr.value[propValue];
+ break;
+ case "string":
+ break;
+ case "undefined":
+ break;
+ case "boolean":
+ break;
+ default:
+ throw "attrValue type can not handle";
@zhizhangchen

zhizhangchen Aug 14, 2012

Contributor

"handle" should be "handled"

Contributor

maxyu commented Aug 15, 2012

Updated.

@zhizhangchen zhizhangchen referenced this pull request Aug 15, 2012

Closed

Integration 20120815 #209

Contributor

grgustaf commented Aug 17, 2012

Merged, thank you.

@grgustaf grgustaf closed this Aug 17, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment