cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shazron <shaz...@gmail.com>
Subject Re: Code review of new ios plugins
Date Tue, 22 Oct 2013 00:14:17 GMT
https://issues.apache.org/jira/browse/CB-5138


On Mon, Oct 21, 2013 at 5:10 PM, Shazron <shazron@gmail.com> wrote:

> Answers inline.
>
>
> Looking at the Keyboard & Statusbar plugins, would you be opposed to using:
>>
>> <clobbers target="cordova.plugins.statusBar" />
>>
>> instead of:
>>
>> <clobbers target="window.StatusBar" />
>>
>>
>
> Agreed.
>
>
>> Would require a major version bump, but probably it has little usage since
>> it's new & in labs still?
>>
>>
> Agreed, major version bump.
>
>
>> Thinking here is that since it's not implementing any spec, we should keep
>> symbols off of window / navigator objects. Would be good to encourage
>> plugins not to pollute the global namespace I think.
>>
>>
> Agreed.
>
>
>> Other points from code-review standpoint:
>> - Why make StatusBar a function? It has no prototype methods.
>>
>
> Originally it had prototype methods, but I changed that - yup we can
> remove that.
>
>
>> - You're calling exec() from the module scope. You should add a <runs/> to
>> the <js-module> to ensure it gets run at start-up. You should also delay
>> deviceready until it executes. cordova-plugin-device has an example of
>> this.
>>
>
> Great - wasn't too sure about how that all worked, let's change that.
>
>
>> - Might be nicer to make the status-bar state a callback with
>> keepCallback=true so that the native code is agnostic to where the
>> namespace is mapped.
>>
>
> Agreed.
>
> Thanks Andrew! I'll add an issue for this.
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message