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

Add custom methods #47

Merged
merged 8 commits into from
Jun 11, 2020
Merged

Add custom methods #47

merged 8 commits into from
Jun 11, 2020

Conversation

divyaashritha
Copy link
Contributor

Summary

Add new CustomMethods object to init and invoke method for both App and frame.
App and frame can call these custom methods on each other and send information without having to explicitly have them declared in XFC.

@divyaashritha
Copy link
Contributor Author

TODO: lint

return Promise.resolve();
};

const obj = Object.assign({}, customMethods, { launch, authorized, unload, resize, event, authorizeConsumer, challengeConsumer, loadPage });

Choose a reason for hiding this comment

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

I know you were trying to just spread your customMethods object into the previous set of methods but it was giving you an unexpected token error like this
image

II think its due to this
esnext/es6-module-transpiler#111
https://stackoverflow.com/questions/10080551/weird-json-parsing-behavior-in-js-unexpected-token
Basically the existing { methods } isn't an object its a block with labels

Aetstetically all these extra const methods seem like noise, can you just do something like

const self = this;
this.JSONRPC = new JSONRPC(
    self.send,
    Object.assign(
        {
            launch() {
                self.wrapper.setAttribute('data-status', 'launched');
                self.emit('xfc.launched');
                return Promise.resolve();
            },

            authorized(detail = {}) {
                self.wrapper.setAttribute('data-status', 'authorized');
                self.emit('xfc.authorized', detail);
                self.initIframeResizer();
                return Promise.resolve();
            },

            unload(detail = {}) {
                self.wrapper.setAttribute('data-status', 'unloaded');
                self.emit('xfc.unload', detail);
                return Promise.resolve();
            },

            resize(height = null, width = null) {
                if (typeof resizeConfig.customCalculationMethod === 'function') {
                resizeConfig.customCalculationMethod.call(self.iframe);
                return Promise.resolve();
                }

                if (height) {
                self.iframe.style.height = height;
                }

                if (width) {
                self.iframe.style.width = width;
                }
                return Promise.resolve();
            },

            event(event, detail) {
                self.emit(event, detail);
                return Promise.resolve();
            },

            authorizeConsumer() {
                return Promise.resolve('hello');
            },

            challengeConsumer() {
                return Promise.resolve(self.secret);
            },

            loadPage(url) {
                self.load(url);
                return Promise.resolve();
            },
        },
        customMethods
    )
);

The syntax validator says it works and then if people add more methods they don't have to remember to add it to this object assign list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will update it!

@roxjcalderon
Copy link
Contributor

Looks like the build is failing?

@@ -57,6 +57,27 @@ describe('Application', () => {
}));
});

describe('#invoke(method, args)', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a test to ensure the custom methods are being setup correctly in JSONRPC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will update!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mhemesath updated!

@@ -2,6 +2,7 @@ Next Release
-------------
1.9.0
------
* Add custom methods #47
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using 1.9.0 as the change was not released

@divyaashritha divyaashritha merged commit 3a10dbe into master Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants