JoshDSommer / nativescript-slides

A NativeScript plugin that is for Intro Tutorials, Image Carousels or any other slide functionality
Other
70 stars 32 forks source link

Events not called #88

Closed noriega9807 closed 7 years ago

noriega9807 commented 7 years ago

Hi, it seems there is an issue regarding the events when working with the component on javascript, the events are not being called (start, changed, cancelled, finished).

I've been trying to get a fix to this with no results. My code is the following: XML:

<Slides:SlideContainer id="slides" row="0" pageIndicators="true" loop="false" start="onStart" cancelled="onCancelled" changed="onChanged" finished="onFinished">
   <Slides:Slide class="slide-1">
    <Label text="Example"  />
    <Label text="lorem ipsum " id="sub1" class="subtext" textWrap="true"/>
    <GridLayout class="nav" rows="auto" columns="auto, auto">
    </GridLayout>
   </Slides:Slide>
   <Slides:Slide class="slide-2">
    <Label text="Example 2"  />
        <Label text="lorem ipsum" id="slide2" class="subtext" textWrap="true"/>
        <GridLayout class="nav" rows="auto" columns="auto, auto">
    </GridLayout>
   </Slides:Slide>
</Slides:SlideContainer>

JS:

var observable = require("data/observable");
var pages = require("ui/page");
var slides =  require('nativescript-slides/nativescript-slides');
var ObservableArray = require("data/observable-array").ObservableArray;
var frameModule =  require('ui/frame');

var slideContainer;

exports.pageLoaded = function(args) {
    var page = args.object;
    page.actionBarHidden = true;
    slideContainer = page.getViewById("slides");
};

exports.onChanged = function(args) {
  alert("fdngjk");
  var data = args.eventData;
    console.log("Changed: " + JSON.stringify(data));
};
toddanglin commented 7 years ago

I noticed this today, too. This used to work, so seems like a regression. Specifically seeing with the 'changed' event, too.

Will look at the source later, but +1 on bug report for now.

toddanglin commented 7 years ago

Okay...so after some quick debugging...problem shows-up in the plugin's demo app, so quickly reproducible. Problem exists on iOS 9/10, so not iOS version specific (didn't check Android). It looks as though the plugin is simply failing to register event observers when defined in the XML.

In other words, events still work if you register handlers manually:

slideContainer.on("changed", (args) => { console.log("Slide changed"); });

But they do not work if assigning handlers from XML:

<slides:SlideContainer changed="onChanged" ...

Fix will require patching this behavior so plugin registers handlers when defined from XML/SlideContainer properties. I've looked at the code, though, and I'm not sure where the problem may lie. Everything looks generally correct, so it's going to take someone more technical to review. :)

@sitefinitysteve Any ideas? I think you originally worked on the event handling code.

sitefinitysteve commented 7 years ago

Offhand no, but I can have a peek tomorrow at work, was planning on being in macos anyway!

On Jan 5, 2017 6:31 PM, "Todd Anglin" notifications@github.com wrote:

Okay...so after some quick debugging...problem shows-up in the plugin's demo app, so quickly reproducible. Problem exists on iOS 9/10, so not iOS version specific (didn't check Android). It looks as though the plugin is simply failing to register event observers when defined in the XML.

In other words, events still work if you register handlers manually:

slideContainer.on("changed", (args) => { console.log("Slide changed"); });

But they do not work if assigning handlers from XML:

<slides:SlideContainer changed="onChanged" ...

Fix will require patching this behavior so plugin registers handlers when defined from XML/SlideContainer properties. I've looked at the code, though, and I'm not sure where the problem may lie. Everything looks generally correct, so it's going to take someone more technical to review. :)

@sitefinitysteve https://github.com/sitefinitysteve Any ideas? I think you originally worked on the event handling code.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TheOriginalJosh/nativescript-slides/issues/88#issuecomment-270778968, or mute the thread https://github.com/notifications/unsubscribe-auth/ABeI6FxeUuPEZvckVGkQP5GM9hxDnfDgks5rPX1ugaJpZM4LaNy9 .

sitefinitysteve commented 7 years ago

Seems to be a change with {NS}... looks like Events now need to have a pattern "nameEvent" so we used to have it like "CHANGED_EVENT" as the static property, just changing to "changedEvent" makes everything work as before...

Finishing testing will do the PR soon

JoshDSommer commented 7 years ago

@sitefinitysteve thanks man!

noriega9807 commented 7 years ago

Thank you @sitefinitysteve ! :)

JoshDSommer commented 7 years ago

released in version 2.2.8