Skip to content
This repository was archived by the owner on Mar 14, 2020. It is now read-only.

Event handler (Regenerate the JS file in ADM)#222

Closed
xuqingkuang wants to merge 3 commits into
intel:masterfrom
xuqingkuang:eventHandlerInADM
Closed

Event handler (Regenerate the JS file in ADM)#222
xuqingkuang wants to merge 3 commits into
intel:masterfrom
xuqingkuang:eventHandlerInADM

Conversation

@xuqingkuang

Copy link
Copy Markdown
Contributor

Event handler use for make user able to edit the events handling codes.

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

The diff with original event handler patch.

diff --git a/src/js/adm.js b/src/js/adm.js
index ea0177f..fd5f136 100644
--- a/src/js/adm.js
+++ b/src/js/adm.js
@@ -2260,6 +2260,11 @@ ADMNode.prototype.setProperty = function (property, value, data, raw) {
         } else {
             this._properties[property] = value;
         }
+
+        // Event handler saving after ID/event property changed.
+        if (property == 'id' || type == 'event')
+            $.rib.saveEventHandlers();
+
         this.fireModelEvent("modelUpdated",
                             { type: "propertyChanged", node: this,
                               property: property, oldValue: orig,
diff --git a/src/js/views/property.js b/src/js/views/property.js
index 47066c5..fb10bc1 100644
--- a/src/js/views/property.js
+++ b/src/js/views/property.js
@@ -106,12 +106,10 @@
             // Use visible attribute to lock the process for make sure it only
             // run 1 time, because _modelUpdatedHandler function will be called
             // 2 times for sync up the widgets in layoutView and codeView.
-            if ((event['property'] == 'id' || event['node'].getType() == 'event')
-                && widget.element.is(':visible')
-            ) {
+            if (event['property'] == 'id' && widget.element.is(':visible')) {
                 // Popup a alert dialog to inform user ID property can't be
                 // blank if it have event handlers.
-                if (event['property'] == 'id' && value.trim() == '') {
+                if (value.trim() == '') {
                     matchedProps = node.getMatchingProperties(
                         {'type': 'event'}
                     );
@@ -142,8 +140,6 @@
                         );
                     };
                 }
-                // Regenerate event handlers file after changed ID/event property.
-                $.rib.saveEventHandlers();
             }

             // Refresh widget, it's necessary to called 2 times for sync up

The patch moved $.rib.saveEventHandlers() from propertyView to ADM, the benefit is make a unitary solution for regenerate the JS file for all ID/event properties operations.

But the defect is because JSONToProj() use setProperty() to initial the project, it's meaning when load a project, it will call many times when load ID/event properties from each widget, maybe parallel write data is concerns for browser LocalStorage implementation, may will caused to some unknown issue.

@zhizhangchen

Copy link
Copy Markdown
Contributor

Why did you put diff in comments? Why not just edit the code directly?

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

The code updated, moved the confirmation code when user clean ID property from _modelUpdateHandler() of propertyView to ADM.

Comment thread src/js/adm.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is "" necessary?

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

The code updated, split jsCode to jsHeader, jsContent and jsFooter.

And added ''value': new RegExp('.+')' to the condition of design.findNodesByProperty() in saveEventHandlers() function.

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

Restored to previous idea, now the main.js file will be regenerated with any condition.

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

Added code for when jsContent is blank then remove the main.js from header and sandbox.

Please review.

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

The code updated, please review.

Thank you @zhizhangchen

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@zhizhangchen The argument has been removed from removeSandboxHeader() and the file will be removed manually, please review.

@zhizhangchen

Copy link
Copy Markdown
Contributor

Setting id of a button to empty doesn't bring up a confirm dialog and event handler stop working after changing button id

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@zhizhangchen The issue is fixed, lacked '!' in haveEventHandlers(), I forgot when it have no event handlers will return true... My mistake.

Thank you for your testing!

@grgustaf

Copy link
Copy Markdown
Contributor

I have some questions about this PR, and in the first place, I'm really wanting to stabilize and just take bug fixes for the next week. So I think we should probably hold off on integrating this.

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@grgustaf and @zhizhangchen, The code has been updated with your comments, please review.

@zhizhangchen

Copy link
Copy Markdown
Contributor

There's confliction between master and this PR. Please rebase it.

Xuqing Kuang added 3 commits August 25, 2012 09:43
For force generate ID property for event handler.
Completed:
1. Event handler dialog construct
2. JS code exporting.
3. JS Preview.
@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@zhizhangchen Rebase completed, please review.

This was referenced Aug 27, 2012
@grgustaf

Copy link
Copy Markdown
Contributor

Finally decided to merge it. It works, there are a few things that need to change, but it's better to get it in people's hands to try out.

Thanks for your patience!

@grgustaf grgustaf closed this Aug 29, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants