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

Export several more missing iOS delegates to allow modification #2827

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
8 participants
@NathanaelA
Contributor

NathanaelA commented Oct 1, 2016

This will allow us to modify the delegates before they are used to add any missing functionality. I have personally ran into this several times, UINavigationController, Label and WebView have all been issues because the delegates are hard coded and so you can't extend them to add new or fix functionality.

The commit message references a specific issue in this repo

Fixes/Implements #2640, #2471

You have [unit tests]

No added unit tests needed; just added the "export" to the typescript code so that the delegates are exported.

NathanaelA added some commits Oct 1, 2016

Several iOS Delegate you are unable to get access to without manually…
… modifying the core modules. The allows you to access the delegate before it is created and then add new prototypes to it to add any missing functionality.
@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 1, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 1, 2016

Can one of the admins verify this patch?

@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 1, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 1, 2016

Can one of the admins verify this patch?

@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 1, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 1, 2016

Can one of the admins verify this patch?

@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 1, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 1, 2016

Can one of the admins verify this patch?

@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 1, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 1, 2016

Can one of the admins verify this patch?

@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 1, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 1, 2016

Can one of the admins verify this patch?

@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 1, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 1, 2016

Can one of the admins verify this patch?

@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 1, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 1, 2016

Can one of the admins verify this patch?

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 1, 2016

Contributor

Bot is apparently going crazy; I guess it REALLY wants someone to verify my patch. ;-)

Contributor

NathanaelA commented Oct 1, 2016

Bot is apparently going crazy; I guess it REALLY wants someone to verify my patch. ;-)

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 3, 2016

Contributor

The CI failing makes no sense the entire patch just adds "exports" to all the delegates that I could find. I'm sure I missed a couple of them, but I hit all the major ones -- especially the three that I have ran into several times over the last 18 months...

Contributor

NathanaelA commented Oct 3, 2016

The CI failing makes no sense the entire patch just adds "exports" to all the delegates that I could find. I'm sure I missed a couple of them, but I hit all the major ones -- especially the three that I have ran into several times over the last 18 months...

@dtopuzov

This comment has been minimized.

Show comment
Hide comment
@dtopuzov

dtopuzov Oct 4, 2016

Member

run ci

Member

dtopuzov commented Oct 4, 2016

run ci

@hshristov

This comment has been minimized.

Show comment
Hide comment
@hshristov

hshristov Oct 5, 2016

Member

Hi @NathanaelA,

exposing delegate implementation is not safe in my opinion. We are using some private fields on the delegates, and some methods must call methods on the owner in order to work correctly. That is why we didn't expose them in the first place. And it make us less flexible - sometimes we change the delegate implementation in order to support different versions and features (e.g. ListView - rowHeight feature use different delegate). Exposing them would make such changes - breaking.

So I think a safer approach is to raise events on the controls that needs customisation through the delegates - like TextEdit, SegmentedBar, etc.

Do you think events will work for your cases?

Member

hshristov commented Oct 5, 2016

Hi @NathanaelA,

exposing delegate implementation is not safe in my opinion. We are using some private fields on the delegates, and some methods must call methods on the owner in order to work correctly. That is why we didn't expose them in the first place. And it make us less flexible - sometimes we change the delegate implementation in order to support different versions and features (e.g. ListView - rowHeight feature use different delegate). Exposing them would make such changes - breaking.

So I think a safer approach is to raise events on the controls that needs customisation through the delegates - like TextEdit, SegmentedBar, etc.

Do you think events will work for your cases?

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 5, 2016

Contributor

Hey @hshristov

Nope, sorry events WON'T work, in the long run imho. If you think about how iOS delegates work, you will see several issue with delegates and events. For you to do events rather than expose the delegates you (as the tns team) would have to override every single delegate option and then raise an event for each one, that would be very quickly unwieldy and the a large chunk of your source would become overriding a delegate functions and firing events and dealing with the return values. Think of the UINavController delegates or WebView delegates, you really want to add over 20 events to each one just so that I can get access to one or two delegate methods that I need? That is a lot of development effort and added code to the ios tns-core-modules side for imho a small gain.

So lets look at real life scenarios -- lets take my #2471- I need access to change: shouldChangeCharactersInRange in side the text delegate. You don't even have it in the prototype chain. In the frame.ios.js, I need access to the NavControllerDelegate for shouldAutoRotate. in the webview delegate I need access to several delegate functions. The issue is that each program/plugin needs totally different delegate access. In some cases this can be easily done because the delegates are already exposed; but in other cases (like the three mentioned above) because they are "private" you make me have to modify the tns-core-modules to expose it, or create some really crazy hacks to get access to it. I HATE having to modify tns-core-modules... ;-) And weird hacks are much more likely to break...

I patch lots of "private" things in the TNS-core-modules; and I do NOT expect them to stay and am willing to (and have) update my plugins when you break something. That is just the risk I take as a plugin author. But when I have access to the private stuff; then I can create the cool plugins like my ns-globalevents, ns-hidden, ns-scaledFonts, ns-dom, ns-orientation, etc. They all tie into the private parts of the framework and modify things to attach there functionality to be able to handle things you didn't either expect to need or create hooks for. And over the last 18 months, I've had to fix a change here and there; but it is well worth it as the abilities I have created in some of my plugins allow me to do things you never envisioned...


Now as to possible solutions:

  1. Expose the delegates; and let me hack them as is. The easiest solution for everyone. It is my responsibility to keep my code working... And if you break it, it is still my responsibility since I am tying into private implementations. ;-) This requires no work on your part, beyond my patch removing the private flag.
  2. Expose events, but this requires a LOT of extra work on your part to add to the event system for each of the delegates and then adding all those missing functions to call events and then handling any return values back to the delegate function back to the framework. This is a lot of work on your part; as you basically have to create delegate events to everything you even think might be used by someone in every single delegate...
  3. Create some sort of delegate subclass system.
    Some sort of delegate registration/init system -- when you include my plugin I can call "global.delegateRegister('text-field', 'UITextFieldDelegate', function(del) {}, priority);" to say that I am subclassing a specific delegate. This way 10 different plugins can each subclass the same delegate and it would still all work (assuming everyone has good code). Then in the text-field.ios.js file; it would do a
    var delegateToUse = global.getDelegate('text-field','UITextFieldDelegate', UITextFieldDelegateImpl);
    The getDelegate code basically passes your UITextFieldDelegateImpl to the first function, its result it passes to the next function until all registered delegate functions are called. Then the final result is passed back to the caller to be used.
  4. Create some sort of delegate mixin system, like the above where you mixin my delegate functions, issues with this idea is that if we both need to have the same function name then I have to make sure MY function has all your code in it to make it work properly.

I think 3 is probably the best idea, as it allows you to keep your delegates private but allows me to override them to do my custom behavior. However, it does require some revamping of a lot of files. So in the meantime until a project can be done for it; I honestly would like the "hack" of 1 being done so I'm not STUCK while waiting for the core to be updated to a delegate sub classing system..

Thoughts?

Contributor

NathanaelA commented Oct 5, 2016

Hey @hshristov

Nope, sorry events WON'T work, in the long run imho. If you think about how iOS delegates work, you will see several issue with delegates and events. For you to do events rather than expose the delegates you (as the tns team) would have to override every single delegate option and then raise an event for each one, that would be very quickly unwieldy and the a large chunk of your source would become overriding a delegate functions and firing events and dealing with the return values. Think of the UINavController delegates or WebView delegates, you really want to add over 20 events to each one just so that I can get access to one or two delegate methods that I need? That is a lot of development effort and added code to the ios tns-core-modules side for imho a small gain.

So lets look at real life scenarios -- lets take my #2471- I need access to change: shouldChangeCharactersInRange in side the text delegate. You don't even have it in the prototype chain. In the frame.ios.js, I need access to the NavControllerDelegate for shouldAutoRotate. in the webview delegate I need access to several delegate functions. The issue is that each program/plugin needs totally different delegate access. In some cases this can be easily done because the delegates are already exposed; but in other cases (like the three mentioned above) because they are "private" you make me have to modify the tns-core-modules to expose it, or create some really crazy hacks to get access to it. I HATE having to modify tns-core-modules... ;-) And weird hacks are much more likely to break...

I patch lots of "private" things in the TNS-core-modules; and I do NOT expect them to stay and am willing to (and have) update my plugins when you break something. That is just the risk I take as a plugin author. But when I have access to the private stuff; then I can create the cool plugins like my ns-globalevents, ns-hidden, ns-scaledFonts, ns-dom, ns-orientation, etc. They all tie into the private parts of the framework and modify things to attach there functionality to be able to handle things you didn't either expect to need or create hooks for. And over the last 18 months, I've had to fix a change here and there; but it is well worth it as the abilities I have created in some of my plugins allow me to do things you never envisioned...


Now as to possible solutions:

  1. Expose the delegates; and let me hack them as is. The easiest solution for everyone. It is my responsibility to keep my code working... And if you break it, it is still my responsibility since I am tying into private implementations. ;-) This requires no work on your part, beyond my patch removing the private flag.
  2. Expose events, but this requires a LOT of extra work on your part to add to the event system for each of the delegates and then adding all those missing functions to call events and then handling any return values back to the delegate function back to the framework. This is a lot of work on your part; as you basically have to create delegate events to everything you even think might be used by someone in every single delegate...
  3. Create some sort of delegate subclass system.
    Some sort of delegate registration/init system -- when you include my plugin I can call "global.delegateRegister('text-field', 'UITextFieldDelegate', function(del) {}, priority);" to say that I am subclassing a specific delegate. This way 10 different plugins can each subclass the same delegate and it would still all work (assuming everyone has good code). Then in the text-field.ios.js file; it would do a
    var delegateToUse = global.getDelegate('text-field','UITextFieldDelegate', UITextFieldDelegateImpl);
    The getDelegate code basically passes your UITextFieldDelegateImpl to the first function, its result it passes to the next function until all registered delegate functions are called. Then the final result is passed back to the caller to be used.
  4. Create some sort of delegate mixin system, like the above where you mixin my delegate functions, issues with this idea is that if we both need to have the same function name then I have to make sure MY function has all your code in it to make it work properly.

I think 3 is probably the best idea, as it allows you to keep your delegates private but allows me to override them to do my custom behavior. However, it does require some revamping of a lot of files. So in the meantime until a project can be done for it; I honestly would like the "hack" of 1 being done so I'm not STUCK while waiting for the core to be updated to a delegate sub classing system..

Thoughts?

@hshristov

This comment has been minimized.

Show comment
Hide comment
@hshristov

hshristov Oct 6, 2016

Member

What about a different approach - you create your delegate that accepts existing delegate. This existing instance will be our implementation without any details exposed. Then in your delegate you handle the method you need and in all other methods you call the default (e.g. our private implementation) delegate. I have this working in the case of ListView (e.g. UITableView delegate).
This will require no change in the modules and most of the time it won't break your plugin.
Will this cover your plugin needs?

And btw - some functionality like visibility='hidden' will be much better in the core-modules :) (e.g. PR)

Member

hshristov commented Oct 6, 2016

What about a different approach - you create your delegate that accepts existing delegate. This existing instance will be our implementation without any details exposed. Then in your delegate you handle the method you need and in all other methods you call the default (e.g. our private implementation) delegate. I have this working in the case of ListView (e.g. UITableView delegate).
This will require no change in the modules and most of the time it won't break your plugin.
Will this cover your plugin needs?

And btw - some functionality like visibility='hidden' will be much better in the core-modules :) (e.g. PR)

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 6, 2016

Contributor

I don't understand what you are saying to do -- Lets take a real case where my patch would help.

How exactly do I get the UINavigationControllerImpl delegate from frame.ios.js?

Line: https://github.com/NativeScript/NativeScript/blob/master/tns-core-modules/ui/frame/frame.ios.ts#L456 is where it is defined (and it isn't exported). So I can't get the delegate there and extend / subclass it.

The next place it is referenced; You actually init it and assign its owner at https://github.com/NativeScript/NativeScript/blob/master/tns-core-modules/ui/frame/frame.ios.ts#L681 -- So at this point you have created the class and initialized it and assigned the delegate to the iOSFrame...

So at what point can I insert/subclass/superclass my code and your delegate?


On the BTW, I totally agree ns-hidden would be much better served being inside the framework. I have several plugins that would make a lot more sense being inside the framework. Issue is I don't particularly like to waste my time to create a PR (like what is happening with this one) and go through the lengthy discussion phase to decide if it will be accepted. It is a hell of a LOT easier for me to just to make a plugin that I can use right now, and skip the entire "lets discuss its merits of the PR"... See the issue I have making enhancement PR's... 😀

Contributor

NathanaelA commented Oct 6, 2016

I don't understand what you are saying to do -- Lets take a real case where my patch would help.

How exactly do I get the UINavigationControllerImpl delegate from frame.ios.js?

Line: https://github.com/NativeScript/NativeScript/blob/master/tns-core-modules/ui/frame/frame.ios.ts#L456 is where it is defined (and it isn't exported). So I can't get the delegate there and extend / subclass it.

The next place it is referenced; You actually init it and assign its owner at https://github.com/NativeScript/NativeScript/blob/master/tns-core-modules/ui/frame/frame.ios.ts#L681 -- So at this point you have created the class and initialized it and assigned the delegate to the iOSFrame...

So at what point can I insert/subclass/superclass my code and your delegate?


On the BTW, I totally agree ns-hidden would be much better served being inside the framework. I have several plugins that would make a lot more sense being inside the framework. Issue is I don't particularly like to waste my time to create a PR (like what is happening with this one) and go through the lengthy discussion phase to decide if it will be accepted. It is a hell of a LOT easier for me to just to make a plugin that I can use right now, and skip the entire "lets discuss its merits of the PR"... See the issue I have making enhancement PR's... 😀

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 8, 2016

Contributor

@hshristov - Any chance you can explain how to do this; I've run into another one of these delegates (TabView) where I have no access to modify the delegate.

Contributor

NathanaelA commented Oct 8, 2016

@hshristov - Any chance you can explain how to do this; I've run into another one of these delegates (TabView) where I have no access to modify the delegate.

@hshristov

This comment has been minimized.

Show comment
Hide comment
@hshristov

hshristov Oct 10, 2016

Member

You implement a delegate like do now and in the constructor you pass an existing delegate. In method implementation you call either the delegate you got in the constructor or you implement it like you need.

pseudo TS code.

class UINavigationControllerAnimatedDelegate extends NSObject implements UINavigationControllerDelegate {
    public static ObjCProtocols = [UINavigationControllerDelegate];
    public static initWithDelegate(defaultImplementation: UINavigationControllerAnimatedDelegate): UINavigationControllerAnimatedDelegate {
        // Init and save defaultImplementation here.
        this.defaultImplementation = defaultImplementation;
    }
    navigationControllerAnimationControllerForOperationFromViewControllerToViewController(params) {
        // either implement the method like you want to or call return this.defaultImplementation. navigationControllerAnimationControllerForOperationFromViewControllerToViewController(params);
    }
}

Then when you want to change the delegate of UINavigationController:

`var frame = new Frame()';
// take current delegate;
var currentDelegate = frame.ios.controller.delegate;
// and pass it to your implementation.
frame.ios.controller.delegate = UINavigationControllerAnimatedDelegate.initWithDelegate(currentDelegate);

Still not bullet-proof but it does not require any changes in modules.
The other approaches that you describe:

  1. Events - I don't want to expose every delegate method as event so my thought was to expose only events that are needed (e.g. on demand).
  2. Some sort of delegate registration - sound interesting. Definitely worth researching.
  3. Delegate mixin - it still tie us to preserve private methods so that delegate could call back to the owner.

In my opinion the safest approach is to create custom class with custom delegate. This way you will have total control. Everything else is not safe enough - we could break it unintentionally which is something I don't like.

Member

hshristov commented Oct 10, 2016

You implement a delegate like do now and in the constructor you pass an existing delegate. In method implementation you call either the delegate you got in the constructor or you implement it like you need.

pseudo TS code.

class UINavigationControllerAnimatedDelegate extends NSObject implements UINavigationControllerDelegate {
    public static ObjCProtocols = [UINavigationControllerDelegate];
    public static initWithDelegate(defaultImplementation: UINavigationControllerAnimatedDelegate): UINavigationControllerAnimatedDelegate {
        // Init and save defaultImplementation here.
        this.defaultImplementation = defaultImplementation;
    }
    navigationControllerAnimationControllerForOperationFromViewControllerToViewController(params) {
        // either implement the method like you want to or call return this.defaultImplementation. navigationControllerAnimationControllerForOperationFromViewControllerToViewController(params);
    }
}

Then when you want to change the delegate of UINavigationController:

`var frame = new Frame()';
// take current delegate;
var currentDelegate = frame.ios.controller.delegate;
// and pass it to your implementation.
frame.ios.controller.delegate = UINavigationControllerAnimatedDelegate.initWithDelegate(currentDelegate);

Still not bullet-proof but it does not require any changes in modules.
The other approaches that you describe:

  1. Events - I don't want to expose every delegate method as event so my thought was to expose only events that are needed (e.g. on demand).
  2. Some sort of delegate registration - sound interesting. Definitely worth researching.
  3. Delegate mixin - it still tie us to preserve private methods so that delegate could call back to the owner.

In my opinion the safest approach is to create custom class with custom delegate. This way you will have total control. Everything else is not safe enough - we could break it unintentionally which is something I don't like.

@valentinstoychev

This comment has been minimized.

Show comment
Hide comment
@valentinstoychev

valentinstoychev Oct 10, 2016

@hshristov @NathanaelA - you folks know each other very well, why not do a quick chat to resolve this :). Just saying :)

valentinstoychev commented Oct 10, 2016

@hshristov @NathanaelA - you folks know each other very well, why not do a quick chat to resolve this :). Just saying :)

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 10, 2016

Contributor

@hshristov I'm up for a chat. Let me know what day & time... I'm on the slack channels; so feel free to ping me there or via skype...

I understood part of what you are saying, and I was afraid that was what you are suggesting; I can think of two issues with this idea

  1. There are several delegate functions that only run when it is created and the delegate is attached; and in two of the cases that I'm discussing these are the ones I need and are accessed way before I can insert my open delegate using the extend method you are proposing.
  2. if I capture the delegate in this fashion; doesn't that mean my delegate has to implement every single one of your delegate functions and pass it on to your delegate. If you later go to add a new delegate function then my plugin will probably break the core modules.
Contributor

NathanaelA commented Oct 10, 2016

@hshristov I'm up for a chat. Let me know what day & time... I'm on the slack channels; so feel free to ping me there or via skype...

I understood part of what you are saying, and I was afraid that was what you are suggesting; I can think of two issues with this idea

  1. There are several delegate functions that only run when it is created and the delegate is attached; and in two of the cases that I'm discussing these are the ones I need and are accessed way before I can insert my open delegate using the extend method you are proposing.
  2. if I capture the delegate in this fashion; doesn't that mean my delegate has to implement every single one of your delegate functions and pass it on to your delegate. If you later go to add a new delegate function then my plugin will probably break the core modules.
@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 13, 2016

Contributor

@hshristov - Any more thoughts on this. I would like to make sure we have this resolved one way or the other by 2.4's release.

Contributor

NathanaelA commented Oct 13, 2016

@hshristov - Any more thoughts on this. I would like to make sure we have this resolved one way or the other by 2.4's release.

@ns-bot

This comment has been minimized.

Show comment
Hide comment
@ns-bot

ns-bot Oct 13, 2016

Can one of the admins verify this patch?

ns-bot commented Oct 13, 2016

Can one of the admins verify this patch?

@hshristov

This comment has been minimized.

Show comment
Hide comment
@hshristov

hshristov Oct 18, 2016

Member

@NathanaelA Well I still don't like the idea of opening all delegates. Too much exposure and while I'm sure you will patch the plugins you do, most people probably won't. So I don't like the idea to introduce breaking changes by exposing internal delegates.

If there are delegate methods that you cannot override because they executed too early we will expose events so that everyone could act on them.

The other suggestion to wrap our delegates (yes you will need to implement all delegate methods but in most of them you will call our delegate) will still work most of the time. When they don't we will review them case by case.

Member

hshristov commented Oct 18, 2016

@NathanaelA Well I still don't like the idea of opening all delegates. Too much exposure and while I'm sure you will patch the plugins you do, most people probably won't. So I don't like the idea to introduce breaking changes by exposing internal delegates.

If there are delegate methods that you cannot override because they executed too early we will expose events so that everyone could act on them.

The other suggestion to wrap our delegates (yes you will need to implement all delegate methods but in most of them you will call our delegate) will still work most of the time. When they don't we will review them case by case.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 18, 2016

Contributor

@hshristov - I have to say I totally disagree with this decision. ;-( The best method imho would be some sort of delegate registry like I mentioned in the comment above #2827 (comment) but that requires additional work to be built and implemented. I was hoping in the meantime that exposing them would have been preferable so that we aren't blocking development, as the decision is now.

However, I have to say this decision wasn't unexpected. I had already brainstormed and come up with an alternative plan of action a while back when I first started discussing this. The tns tool chain has a cool nativescript-hook publish step, I can write a simple hook plugin tns-core-patcher that adds all these "exports.x" to all these files manually during the build and it would eliminate any issue for anyone using any of my plugins when I need to patch one of these delegates... Any plugin I write that requires this functionality will just depend on the new tns-core-patcher plugin as a dependency.

Since it is hook plugin it will allow everything to continue working even when the core modules are updated and doesn't require install or any other work by users; and the end results acts just like you exposed them for me by accepting this patch would have done. 😀

Contributor

NathanaelA commented Oct 18, 2016

@hshristov - I have to say I totally disagree with this decision. ;-( The best method imho would be some sort of delegate registry like I mentioned in the comment above #2827 (comment) but that requires additional work to be built and implemented. I was hoping in the meantime that exposing them would have been preferable so that we aren't blocking development, as the decision is now.

However, I have to say this decision wasn't unexpected. I had already brainstormed and come up with an alternative plan of action a while back when I first started discussing this. The tns tool chain has a cool nativescript-hook publish step, I can write a simple hook plugin tns-core-patcher that adds all these "exports.x" to all these files manually during the build and it would eliminate any issue for anyone using any of my plugins when I need to patch one of these delegates... Any plugin I write that requires this functionality will just depend on the new tns-core-patcher plugin as a dependency.

Since it is hook plugin it will allow everything to continue working even when the core modules are updated and doesn't require install or any other work by users; and the end results acts just like you exposed them for me by accepting this patch would have done. 😀

@PeterStaev

This comment has been minimized.

Show comment
Hide comment
@PeterStaev

PeterStaev Oct 19, 2016

Contributor

I tend to agree with this one with @NathanaelA and I know how he feels as I had to cope with this in a few situations as well. I ended up copy-paste the implementation from the core modules in my code and adding whatever additional I needed (example)...

@hshristov, the reasoning that changing the internal implementation might break plugins/code is also valid, but even with your suggestion to create a wrapper delegate, if you make breaking changes to the original delegate, it will probably break this kind of implementation as well. Same goes for my workaround of copy-paste implementation. But with having delegates exported, it will make code a lot more readable and easy to change in case of a breaking change in the base implementation.

Contributor

PeterStaev commented Oct 19, 2016

I tend to agree with this one with @NathanaelA and I know how he feels as I had to cope with this in a few situations as well. I ended up copy-paste the implementation from the core modules in my code and adding whatever additional I needed (example)...

@hshristov, the reasoning that changing the internal implementation might break plugins/code is also valid, but even with your suggestion to create a wrapper delegate, if you make breaking changes to the original delegate, it will probably break this kind of implementation as well. Same goes for my workaround of copy-paste implementation. But with having delegates exported, it will make code a lot more readable and easy to change in case of a breaking change in the base implementation.

@hshristov

This comment has been minimized.

Show comment
Hide comment
@hshristov

hshristov Oct 19, 2016

Member

@NathanaelA And I totally disagree with your decision. Patching tns-core-modules with plugin makes them unsupportable so anyone using such plugin will have hard time to explain which version they are using, which version of the patch plugin and so on.
Delegate registration still won't work because we have some logic in the delegate that needs to call to delegate owner (like in UITextFieldDelegateImpl). We don't have a way to force the delegate implementation to call these methods and without calling them the control won't work.

@PeterStaev My suggestion for wrapping existing delegate is temporary until we know which cases are not possible right now. Please list the cases that you need to modify/extend current delegate implementation so that we plan the changes for the next version.

Just to make sure we are on the same side - I want to make modules as open as possible but still keeps them solid. Exposing private/internal parts of implementation is against all good coding policy. We have to still enhance and fix existing implementation, support iOS 8, 9, 10 and so on.

Lets discuss the delegates you guys need and make them cover your cases.

Member

hshristov commented Oct 19, 2016

@NathanaelA And I totally disagree with your decision. Patching tns-core-modules with plugin makes them unsupportable so anyone using such plugin will have hard time to explain which version they are using, which version of the patch plugin and so on.
Delegate registration still won't work because we have some logic in the delegate that needs to call to delegate owner (like in UITextFieldDelegateImpl). We don't have a way to force the delegate implementation to call these methods and without calling them the control won't work.

@PeterStaev My suggestion for wrapping existing delegate is temporary until we know which cases are not possible right now. Please list the cases that you need to modify/extend current delegate implementation so that we plan the changes for the next version.

Just to make sure we are on the same side - I want to make modules as open as possible but still keeps them solid. Exposing private/internal parts of implementation is against all good coding policy. We have to still enhance and fix existing implementation, support iOS 8, 9, 10 and so on.

Lets discuss the delegates you guys need and make them cover your cases.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 19, 2016

Contributor

@hshristov - Oh, I bet you don't like my "final" solution. I don't really like it either. 😀 I totally realize the pitfalls it opens up; and unfortunately for you(Telerik); Telerik will totally get blamed if it breaks anything at any time... (Especially since it only modifies the platform versions...) I fully understand why it is the worst solution of the bunch; but my point is; if that is the only solution to go forward; then unfortunately I have very little choice other than to use it. I get to choose between access or no access; I'm always going to choose access, so that I can get my job done. I didn't toss it into the discussion earlier because it really is a method of last resort, and was not a route I wanted to take (because of the pitfalls), and so I had hoped to come to some sort of agreement that would work for everyone, before having to use my final failsafe method. 😀

As for a delegate registry; I don't see anything in UITextFieldDelegateImpl that would block the idea of a delegate registration system. The idea of a delegate registration system would basically allow me to subclass the delegates BEFORE it is actually created so that any function I am overriding I would call super on. This allows every single delegate function registered in the system for that delegate type to run. It allows ALL of our delegate functions in the chain to work including your ability to call back to the owner to do your things. Your delegate function would be just like mine, somewhere in the super chain... Technically it isn't a very complicated issue to implement the logistics of that part of the piece. However, the piece that is complicated which we do have to figure out in this idea is how to setup all the registrations during a very early point of bootstrap; before ANY delegates are actually created, since you have to have them registered to actually set them up. Since we really don't want to scan every plugin folder during runtime at bootup. We probably have to have an actual build step that combines all the plugins delegate.registration.js files into a single file or something that can make this a single file that the runtime can load from the main tns-core during startup. I honestly thing this is the best way to implement this and solve all of our issues. But it requires putting some engineering work into it from several points which means it will take probably a couple versions before it works well. In the meantime I don't want to be blocked while we figure this out, so opening up access right now is my stopgap measure until a final solution is developed. We can depreciate the direct access and re-make them all private when we have a final solution.


Just to make sure we are on the same side - I want to make modules as open as possible but still keeps them solid. Exposing private/internal parts of implementation is against all good coding policy. We have to still enhance and fix existing implementation, support iOS 8, 9, 10 and so on.

I am fully aware of this issue from your point of view; but as a plugin developer I also have to deal with the exact same issues. If my plugin breaks because you modified an internal delegate; then it is my responsibility to fix it, not yours. I've had to fix my ns-orientation, and ns-dom at least once or twice over the TNS core upgrades because you fixed bugs and/or completely changed internal and even once you eliminated/modified a public routines that I was depending on. This is to be expected, if I use internal calls; I expect to have to fix them when they are broken/changed/removed.

However, on the flip side I think every single one of my projects uses my NS-Dom library (& my liveedit), and half of them use my ns-orientation plugin; and a whole slew of my other plugins are in several of my projects... I wouldn't ever want to live without most of my plugins; they provide me a lot more flexibility that the default platform. So they will get fixed...


Lets discuss the delegates you guys need and make them cover your cases.

This idea really really works very poorly for me... The TNS releases are typically somewhere around 5+ weeks. (We are already at 48 days for 2.4 and it looks like it will be >60 days now). In addition if my request/fix comes in too late in the cycle it is pushed to the next release. This is extremely horrible when we are trying to get things done, to have to wait forever before we can use it. When I need it; it isn't that I need it in a couple months, it is I needed it yesterday because I'm creating a plugin to fix a clients need right now. I can't tell my clients, oh yeah we "hope" it will be fixed in a couple months, that just doesn't fly. ;-(

Oh and if you are thinking of bringing up the @next versions -- the @next's are not really a useful solution for actual clients; they tend to be buggy and they are "not" really the next version, they are the master version meaning they frequently contain code that is not going to be in a real release version until (hopefully) some distant future version. If you were to go to a weekly next version with primarily just bug fixes and smaller changes; rather than the current master version that would go a long ways to make this more of a non-issue for me.

So there was a reason I exported all of these delegates. I may NEVER use most of them, but I when I need them, I don't want to be begging to "maybe" see them in a couple months...

Contributor

NathanaelA commented Oct 19, 2016

@hshristov - Oh, I bet you don't like my "final" solution. I don't really like it either. 😀 I totally realize the pitfalls it opens up; and unfortunately for you(Telerik); Telerik will totally get blamed if it breaks anything at any time... (Especially since it only modifies the platform versions...) I fully understand why it is the worst solution of the bunch; but my point is; if that is the only solution to go forward; then unfortunately I have very little choice other than to use it. I get to choose between access or no access; I'm always going to choose access, so that I can get my job done. I didn't toss it into the discussion earlier because it really is a method of last resort, and was not a route I wanted to take (because of the pitfalls), and so I had hoped to come to some sort of agreement that would work for everyone, before having to use my final failsafe method. 😀

As for a delegate registry; I don't see anything in UITextFieldDelegateImpl that would block the idea of a delegate registration system. The idea of a delegate registration system would basically allow me to subclass the delegates BEFORE it is actually created so that any function I am overriding I would call super on. This allows every single delegate function registered in the system for that delegate type to run. It allows ALL of our delegate functions in the chain to work including your ability to call back to the owner to do your things. Your delegate function would be just like mine, somewhere in the super chain... Technically it isn't a very complicated issue to implement the logistics of that part of the piece. However, the piece that is complicated which we do have to figure out in this idea is how to setup all the registrations during a very early point of bootstrap; before ANY delegates are actually created, since you have to have them registered to actually set them up. Since we really don't want to scan every plugin folder during runtime at bootup. We probably have to have an actual build step that combines all the plugins delegate.registration.js files into a single file or something that can make this a single file that the runtime can load from the main tns-core during startup. I honestly thing this is the best way to implement this and solve all of our issues. But it requires putting some engineering work into it from several points which means it will take probably a couple versions before it works well. In the meantime I don't want to be blocked while we figure this out, so opening up access right now is my stopgap measure until a final solution is developed. We can depreciate the direct access and re-make them all private when we have a final solution.


Just to make sure we are on the same side - I want to make modules as open as possible but still keeps them solid. Exposing private/internal parts of implementation is against all good coding policy. We have to still enhance and fix existing implementation, support iOS 8, 9, 10 and so on.

I am fully aware of this issue from your point of view; but as a plugin developer I also have to deal with the exact same issues. If my plugin breaks because you modified an internal delegate; then it is my responsibility to fix it, not yours. I've had to fix my ns-orientation, and ns-dom at least once or twice over the TNS core upgrades because you fixed bugs and/or completely changed internal and even once you eliminated/modified a public routines that I was depending on. This is to be expected, if I use internal calls; I expect to have to fix them when they are broken/changed/removed.

However, on the flip side I think every single one of my projects uses my NS-Dom library (& my liveedit), and half of them use my ns-orientation plugin; and a whole slew of my other plugins are in several of my projects... I wouldn't ever want to live without most of my plugins; they provide me a lot more flexibility that the default platform. So they will get fixed...


Lets discuss the delegates you guys need and make them cover your cases.

This idea really really works very poorly for me... The TNS releases are typically somewhere around 5+ weeks. (We are already at 48 days for 2.4 and it looks like it will be >60 days now). In addition if my request/fix comes in too late in the cycle it is pushed to the next release. This is extremely horrible when we are trying to get things done, to have to wait forever before we can use it. When I need it; it isn't that I need it in a couple months, it is I needed it yesterday because I'm creating a plugin to fix a clients need right now. I can't tell my clients, oh yeah we "hope" it will be fixed in a couple months, that just doesn't fly. ;-(

Oh and if you are thinking of bringing up the @next versions -- the @next's are not really a useful solution for actual clients; they tend to be buggy and they are "not" really the next version, they are the master version meaning they frequently contain code that is not going to be in a real release version until (hopefully) some distant future version. If you were to go to a weekly next version with primarily just bug fixes and smaller changes; rather than the current master version that would go a long ways to make this more of a non-issue for me.

So there was a reason I exported all of these delegates. I may NEVER use most of them, but I when I need them, I don't want to be begging to "maybe" see them in a couple months...

@PeterStaev

This comment has been minimized.

Show comment
Hide comment
@PeterStaev

PeterStaev Oct 19, 2016

Contributor

@hshristov , the ones that pop up in my mind are UIPickerViewDelegateImpl and UIScrollViewDelegateImpl

Contributor

PeterStaev commented Oct 19, 2016

@hshristov , the ones that pop up in my mind are UIPickerViewDelegateImpl and UIScrollViewDelegateImpl

@hshristov

This comment has been minimized.

Show comment
Hide comment
@hshristov

hshristov Oct 19, 2016

Member

@PeterStaev I mean what is the case that you need to implement, which delegates and methods you need?

@NathanaelA As I said before there is another option - to create custom UI component. This way you will manage the native view, the delegate, everything. We won't break you when we change something.

But again as I said I want to make modules as useful as possible so if you list all scenarios that need custom delegates we might be able to handle them without the need of any hack.

Member

hshristov commented Oct 19, 2016

@PeterStaev I mean what is the case that you need to implement, which delegates and methods you need?

@NathanaelA As I said before there is another option - to create custom UI component. This way you will manage the native view, the delegate, everything. We won't break you when we change something.

But again as I said I want to make modules as useful as possible so if you list all scenarios that need custom delegates we might be able to handle them without the need of any hack.

@hamorphis

This comment has been minimized.

Show comment
Hide comment
@hamorphis

hamorphis Oct 19, 2016

Contributor

@NathanaelA What about adding the missing functionality to the delegates directly in tns-core-modules. In this way, the default views in tns-core-modules will benefit from your enhancements and will improve NativeScript for all people that are using it.

Contributor

hamorphis commented Oct 19, 2016

@NathanaelA What about adding the missing functionality to the delegates directly in tns-core-modules. In this way, the default views in tns-core-modules will benefit from your enhancements and will improve NativeScript for all people that are using it.

@PeterStaev

This comment has been minimized.

Show comment
Hide comment
@PeterStaev

PeterStaev Oct 19, 2016

Contributor

@hshristov , for the picker one I needed it, because I wanted to format the text in the picker. Here is again the link to the code: https://github.com/PeterStaev/NativeScript-Drop-Down/blob/master/drop-down.ios.ts#L147. For the scrollview I needed to implement some custom logic for pageable scrolling in addition to the default event notification.

Contributor

PeterStaev commented Oct 19, 2016

@hshristov , for the picker one I needed it, because I wanted to format the text in the picker. Here is again the link to the code: https://github.com/PeterStaev/NativeScript-Drop-Down/blob/master/drop-down.ios.ts#L147. For the scrollview I needed to implement some custom logic for pageable scrolling in addition to the default event notification.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 19, 2016

Contributor

@hamorphis - I know I tend to write books in response; but I did answer this issue above in my other response to @hshristov . ;-)

  1. I'm actually all for adding missing functionality to the framework (when it needs it). However, some of the plugins (like forcing the screen orientation programmatically), is questionable if it is actually needed in the core framework or better served as a plugin. It isn't something used very much. In fact I haven't used it in any of my apps; but a client needs it for his app. This is a real example that I have had to hack around and the best solution is locked behind one of those stupid "private" delegates that I'm trying to free with this patch. It cost me MANY MANY hours to bypass the chain in a way that worked reliably, where directly/manually modifying the delegate source prototype inside tns-core-modules worked immediately.
  2. The NS releases are slow; 5+ weeks between releases is a killer. If the patch is too late in the cycle; make that over 2 months wait before it shows up... This does not work for clients who are paying for a plugin to say; sorry 2.3 doesn't offer it; 2.5 might, see you next January! I don't really want to start distributing customized tns-core-modules if I can avoid it... To me if the framework stands in my way, then creating a plugin that re-writes the core-modules and fixes the framework is a way better option for me then me distributing customized tns-core-modules to a plugin clients. At least rewriting the code should continue to work when you release 2.4, 2.5, etc...
  3. There is no guarantee a patch of mine will make it into core; take this one for example. I have outlined several times WHY I need this ability (with actual REAL use cases); and I understand your reluctance (don't agree with it, but understand it) to implement it. Yet, you haven't offered me any workable solution in this thread. I can't wrap the app delegate; that is frequently too late in the component lifecycle -- the functions I need to wrap will have already been called by iOS when the component was initialized. And waiting 3-8 weeks to MAYBE (again, no guarantees a patch will be accepted) to let my client have a plugin I wrote, doesn't work very well either for me to get paid.

In summary; the framework needs to be enabling me; not standing in my way... If it stands in my way, I will beat it down with a 🔨 And yes, no one may like the solution, but at least it is a solution. 😀

Contributor

NathanaelA commented Oct 19, 2016

@hamorphis - I know I tend to write books in response; but I did answer this issue above in my other response to @hshristov . ;-)

  1. I'm actually all for adding missing functionality to the framework (when it needs it). However, some of the plugins (like forcing the screen orientation programmatically), is questionable if it is actually needed in the core framework or better served as a plugin. It isn't something used very much. In fact I haven't used it in any of my apps; but a client needs it for his app. This is a real example that I have had to hack around and the best solution is locked behind one of those stupid "private" delegates that I'm trying to free with this patch. It cost me MANY MANY hours to bypass the chain in a way that worked reliably, where directly/manually modifying the delegate source prototype inside tns-core-modules worked immediately.
  2. The NS releases are slow; 5+ weeks between releases is a killer. If the patch is too late in the cycle; make that over 2 months wait before it shows up... This does not work for clients who are paying for a plugin to say; sorry 2.3 doesn't offer it; 2.5 might, see you next January! I don't really want to start distributing customized tns-core-modules if I can avoid it... To me if the framework stands in my way, then creating a plugin that re-writes the core-modules and fixes the framework is a way better option for me then me distributing customized tns-core-modules to a plugin clients. At least rewriting the code should continue to work when you release 2.4, 2.5, etc...
  3. There is no guarantee a patch of mine will make it into core; take this one for example. I have outlined several times WHY I need this ability (with actual REAL use cases); and I understand your reluctance (don't agree with it, but understand it) to implement it. Yet, you haven't offered me any workable solution in this thread. I can't wrap the app delegate; that is frequently too late in the component lifecycle -- the functions I need to wrap will have already been called by iOS when the component was initialized. And waiting 3-8 weeks to MAYBE (again, no guarantees a patch will be accepted) to let my client have a plugin I wrote, doesn't work very well either for me to get paid.

In summary; the framework needs to be enabling me; not standing in my way... If it stands in my way, I will beat it down with a 🔨 And yes, no one may like the solution, but at least it is a solution. 😀

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 19, 2016

Contributor

@hshristov - Your suggestion of creating my own component is also a non-starter in a some of these cases. I really have NO issue creating my own components, I'm up to somewhere around 26+ public plugins and almost as many private... ;-)

Lets go BACK to my REAL use case... Which I have provided above, so we have something to talk about that is REAL. I can't really create a new TNSApplication component for iOS, do you realize how deeply embedded this class is in the entire TNS core modules. Like I mentioned above I need access to the app delegate before it is created.

I guess technically I could copy the TNS application.ios.js code into my own file; make my 10 lines of code changes to it then require the client to call my require ("ns-mt-application") as the very first line of app.js/app.ts/main.ts file, which would then swap out the entire tns core modules application variable for mine, so that it is running my code and then my delegate. BUT when you go to release 2.4 or 2.5, the odds of something breaking are pretty high. This seems like a huge non-starter to me. ;-)

It would be way more easy and safe to just create/fork my own tns-core-modules with all my patches and distribute that as a requirement for all my plugins. I have so far attempted to NOT fork the core-modules; but I guess it is technically a valid option... It would for sure simplify some things drastically for me, and might be valuable to a lot of people since simple bug fixes could be released quickly... Hmm, I'll have to seriously think about that option, I already have all the infrastructure to do it...

Back to your original idea, my objective is not to be maintaining my own TNSApp class for each release. My objective is to release a plugin that I hopefully never have to touch again because it is solid and will continue to work. ;-) That is why I patch my own prototype into the delegate.


So I think these are all the potentially valid cases provided so far:

  1. exporting the delegates (this patch)
  2. a delegate registration system (a lot of infrastructure has to be done)
  3. tns-core-patcher (potentially can mess up platform/tns-core files, Telerik blamed)
  4. Fork tns-core-modules (Might split the community, or might be empowering like io.js)
  5. Making my own component (A lot of work to just add 5-10 lines of code, but could be valid in some cases)
  6. Wrapping the already created delegate. (Timing failure, more memory, and changes to core delegate code will break the plugin)

Any other ideas?

Contributor

NathanaelA commented Oct 19, 2016

@hshristov - Your suggestion of creating my own component is also a non-starter in a some of these cases. I really have NO issue creating my own components, I'm up to somewhere around 26+ public plugins and almost as many private... ;-)

Lets go BACK to my REAL use case... Which I have provided above, so we have something to talk about that is REAL. I can't really create a new TNSApplication component for iOS, do you realize how deeply embedded this class is in the entire TNS core modules. Like I mentioned above I need access to the app delegate before it is created.

I guess technically I could copy the TNS application.ios.js code into my own file; make my 10 lines of code changes to it then require the client to call my require ("ns-mt-application") as the very first line of app.js/app.ts/main.ts file, which would then swap out the entire tns core modules application variable for mine, so that it is running my code and then my delegate. BUT when you go to release 2.4 or 2.5, the odds of something breaking are pretty high. This seems like a huge non-starter to me. ;-)

It would be way more easy and safe to just create/fork my own tns-core-modules with all my patches and distribute that as a requirement for all my plugins. I have so far attempted to NOT fork the core-modules; but I guess it is technically a valid option... It would for sure simplify some things drastically for me, and might be valuable to a lot of people since simple bug fixes could be released quickly... Hmm, I'll have to seriously think about that option, I already have all the infrastructure to do it...

Back to your original idea, my objective is not to be maintaining my own TNSApp class for each release. My objective is to release a plugin that I hopefully never have to touch again because it is solid and will continue to work. ;-) That is why I patch my own prototype into the delegate.


So I think these are all the potentially valid cases provided so far:

  1. exporting the delegates (this patch)
  2. a delegate registration system (a lot of infrastructure has to be done)
  3. tns-core-patcher (potentially can mess up platform/tns-core files, Telerik blamed)
  4. Fork tns-core-modules (Might split the community, or might be empowering like io.js)
  5. Making my own component (A lot of work to just add 5-10 lines of code, but could be valid in some cases)
  6. Wrapping the already created delegate. (Timing failure, more memory, and changes to core delegate code will break the plugin)

Any other ideas?

@hshristov

This comment has been minimized.

Show comment
Hide comment
@hshristov

hshristov Oct 20, 2016

Member

@NathanaelA We already support adding custom application delegate:

    class AppDelegate extends NSObject implements UIApplicationDelegate {
        static ObjCProtocols = [UIApplicationDelegate];
        applicationHandleOpenURL(app, url): boolean {
            // Your code here.
        }
    }
    application.ios.delegate = AppDelegate;

The other cases if I understand correctly is ScrollChanged event on the scrollView.
If would be best if you submit PR directly with the changes you need to make in order to support your scenario or at least open an issue and describe them so we could think of the best way to implement it.

Member

hshristov commented Oct 20, 2016

@NathanaelA We already support adding custom application delegate:

    class AppDelegate extends NSObject implements UIApplicationDelegate {
        static ObjCProtocols = [UIApplicationDelegate];
        applicationHandleOpenURL(app, url): boolean {
            // Your code here.
        }
    }
    application.ios.delegate = AppDelegate;

The other cases if I understand correctly is ScrollChanged event on the scrollView.
If would be best if you submit PR directly with the changes you need to make in order to support your scenario or at least open an issue and describe them so we could think of the best way to implement it.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 20, 2016

Contributor

@hshristov

Sorry my mistake I referred to the wrong delegate. To many delegates. 😀 NavControllerDelegate (ie. the UINavigationControllerImpl in frame.ios.js) is the delegate I need access to. However, based on the init cycle this gets created when the app is created; I need access to change the delegate BEFORE the app creates the delegate. Also if I recall correctly, one of the ios TabBar delegates; I'm pretty sure I need before it is created also as it does some initial sizing of parts of the tab bar code when created...

If you are opposed to this pull request, then just please close it (& #2471)-- me arguing for it is pointless for both of us -- if there isn't a chance it will be accepted. 😀

You know my position, and what I believe needs to be done, if you decide in the future to implement the delegate registry system correctly, then that would solve the issue too. Otherwise, we are just wasting each others time, and we both would be better off working on the other projects that we are working on right now... 😀

P.S. I don't Believe I have any scroll view cases; my cases off the top of my head have been so far:

  1. Orientation locking (Worked around, but very hacky imho)
  2. Text Entry size locking (Not fixed, if I recall correctly)
  3. TabView sizing (Don't recall if I have fixed it)
  4. WebKit customizations. (manually exported it, if I recall correctly)
Contributor

NathanaelA commented Oct 20, 2016

@hshristov

Sorry my mistake I referred to the wrong delegate. To many delegates. 😀 NavControllerDelegate (ie. the UINavigationControllerImpl in frame.ios.js) is the delegate I need access to. However, based on the init cycle this gets created when the app is created; I need access to change the delegate BEFORE the app creates the delegate. Also if I recall correctly, one of the ios TabBar delegates; I'm pretty sure I need before it is created also as it does some initial sizing of parts of the tab bar code when created...

If you are opposed to this pull request, then just please close it (& #2471)-- me arguing for it is pointless for both of us -- if there isn't a chance it will be accepted. 😀

You know my position, and what I believe needs to be done, if you decide in the future to implement the delegate registry system correctly, then that would solve the issue too. Otherwise, we are just wasting each others time, and we both would be better off working on the other projects that we are working on right now... 😀

P.S. I don't Believe I have any scroll view cases; my cases off the top of my head have been so far:

  1. Orientation locking (Worked around, but very hacky imho)
  2. Text Entry size locking (Not fixed, if I recall correctly)
  3. TabView sizing (Don't recall if I have fixed it)
  4. WebKit customizations. (manually exported it, if I recall correctly)
@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Oct 30, 2016

Contributor

I have decided to start down the TNS-Core-Patcher plugin idea... The more I think about it; the more I realized it would totally solve all my patch problems with the team. 😀

I can build them, and they can be applied by TCP and then if you decide you want them built into the engine you will see the benefit because of the usage of the plugin that needs it, you will then have real data on how much usage it gets -- which will allow you to make a real decision if it is benefits the core modules, rather than arbitrarily dismissing patches because you don't see the benefit... 😀

Please feel free to comment on the repo, for any safety or issue you want me to try and take in account. My objective isn't to break anything; and I've been brainstorming how to do this safely; and have a issue relating to this subject on my repo...

😀

Contributor

NathanaelA commented Oct 30, 2016

I have decided to start down the TNS-Core-Patcher plugin idea... The more I think about it; the more I realized it would totally solve all my patch problems with the team. 😀

I can build them, and they can be applied by TCP and then if you decide you want them built into the engine you will see the benefit because of the usage of the plugin that needs it, you will then have real data on how much usage it gets -- which will allow you to make a real decision if it is benefits the core modules, rather than arbitrarily dismissing patches because you don't see the benefit... 😀

Please feel free to comment on the repo, for any safety or issue you want me to try and take in account. My objective isn't to break anything; and I've been brainstorming how to do this safely; and have a issue relating to this subject on my repo...

😀

@valentinstoychev

This comment has been minimized.

Show comment
Hide comment
@valentinstoychev

valentinstoychev Nov 22, 2016

Sometimes it is hard to come to a conclusion that suits everyone. This is one of these cases. I'm closing this request after I discussed with both Nathanael and the core team.

valentinstoychev commented Nov 22, 2016

Sometimes it is hard to come to a conclusion that suits everyone. This is one of these cases. I'm closing this request after I discussed with both Nathanael and the core team.

@speigg

This comment has been minimized.

Show comment
Hide comment
@speigg

speigg Jan 4, 2018

Contributor

@valentinstoychev I ran into this issue as well, trying to add preferredScreenEdgesDeferringSystemGestures to the Page's UIViewController. It seems like there isn't a good solution for these kinds of issues right now.

I understand that new javascript methods can't be added to a Obj-C subclass after the extend method is called... however, if this limitation were reversed with a new feature (global.__updateNativeClass?), then I could simply do the following:

const viewController = <UIVIewController>page.ios;
const viewControllerConstructor = viewController.constructor;
const viewControllerPrototype = viewController.constructor.prototype;
viewControllerPrototype.preferredScreenEdgesDeferringSystemGestures = () => UIRectEdge.All;
global.__updateNativeClass(viewControllerConstructor); // update ObjC class with new js methods
viewController.setNeedsUpdateOfScreenEdgesDeferringSystemGestures();

Wouldn't this solve everyone's problems?

Contributor

speigg commented Jan 4, 2018

@valentinstoychev I ran into this issue as well, trying to add preferredScreenEdgesDeferringSystemGestures to the Page's UIViewController. It seems like there isn't a good solution for these kinds of issues right now.

I understand that new javascript methods can't be added to a Obj-C subclass after the extend method is called... however, if this limitation were reversed with a new feature (global.__updateNativeClass?), then I could simply do the following:

const viewController = <UIVIewController>page.ios;
const viewControllerConstructor = viewController.constructor;
const viewControllerPrototype = viewController.constructor.prototype;
viewControllerPrototype.preferredScreenEdgesDeferringSystemGestures = () => UIRectEdge.All;
global.__updateNativeClass(viewControllerConstructor); // update ObjC class with new js methods
viewController.setNeedsUpdateOfScreenEdgesDeferringSystemGestures();

Wouldn't this solve everyone's problems?

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Jan 5, 2018

Contributor

For anyone interested in a solution to this artificial door. @speigg actually caused me to see a open window to allow you to be able to do this now via his 850 issue. His post above actually has all the relevant components needed. You just need access to the prototype; and he is correct you can access via the constructor.prototype or my method I detail in this blog post.
-= Blog link removed since it actually doesn't work =-

Contributor

NathanaelA commented Jan 5, 2018

For anyone interested in a solution to this artificial door. @speigg actually caused me to see a open window to allow you to be able to do this now via his 850 issue. His post above actually has all the relevant components needed. You just need access to the prototype; and he is correct you can access via the constructor.prototype or my method I detail in this blog post.
-= Blog link removed since it actually doesn't work =-

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Jan 10, 2018

Contributor

Unfortunately a different bug in the iOS version of NativeScript prevents the ability I outlined in my blog post. (Talk about seriously disappointing!) Odds of getting that bug fixed; I'd wager are about as high as the Pope renouncing the Catholic church. So we are still stuck with hand modifying the core modules to get access to any delegates.

Contributor

NathanaelA commented Jan 10, 2018

Unfortunately a different bug in the iOS version of NativeScript prevents the ability I outlined in my blog post. (Talk about seriously disappointing!) Odds of getting that bug fixed; I'd wager are about as high as the Pope renouncing the Catholic church. So we are still stuck with hand modifying the core modules to get access to any delegates.

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