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

Navigation.showError() modals are unable to be dismissed #52

Closed
dnicolson opened this issue Feb 1, 2020 · 6 comments · May be fixed by #56
Closed

Navigation.showError() modals are unable to be dismissed #52

dnicolson opened this issue Feb 1, 2020 · 6 comments · May be fixed by #56

Comments

@dnicolson
Copy link

Modal dialogs created with navigationDocument.presentModal() are able to be dismissed but using Navigation.showError() results in a dialog where the button does not work, escape or home must be used to dismiss.

Consider the following code if added to the Movie Catalog example app in the ready method of the HomePage object:

const showError = () => {
  // const alert = `<?xml version="1.0" encoding="UTF-8" ?>
  //     <document>
  //       <alertTemplate>
  //         <title>Error</title>
  //         <description>This modal cannot be dismissed.</description>
  //         <button data-alert-dissmiss="close">
  //           <text>OK</text>
  //         </button>
  //       </alertTemplate>
  //     </document>`;
  // const error = ATV.Navigation.showError({ template: alert, style: '' });

  const alert = `<?xml version="1.0" encoding="UTF-8" ?>
    <document>
      <alertTemplate>
        <title>Error</title>
        <description>This modal can be dismissed.</description>
        <button>
          <text>OK</text>
        </button>
      </alertTemplate>
    </document>`;
  const parser = new DOMParser();
  const error = parser.parseFromString(alert, 'application/xml');
  navigationDocument.presentModal(error);

  error.addEventListener('select', () => {
    ATV.Navigation.dismissModal();
  })
};

The uncommented version works as expected. Inverting the commented code results in the modal not being dismissed but with the data-alert-dissmiss [sic] attribute the navigationDocument.dismissModal() method is called, as indicated by the log:
close button clicked within the modal, dismissing modal...

@emadalam
Copy link
Owner

emadalam commented Feb 2, 2020

@dnicolson Can you give me a working fork somewhere to test this behaviour?

Some things to help you further.

  • Your templates don't need <?xml version="1.0" encoding="UTF-8" ?>, you can use ATV.Page.create or the raw ATV.Parser.makeDom
  • The dom needs to be created using the atv js methods in order to have the global handlers to work
  • Have you tried using a global template for error?
const errorTpl = (data) => `
<document>
  <descriptiveAlertTemplate>
    <title>${data.title}</title>
    <description>${data.message}</description>
    <button data-alert-dissmiss="true">
      <text>Ok</text>
    </button>
  </descriptiveAlertTemplate>
</document>`;

ATV.start({
  menu: {
    // menu items
  },
  templates: {
    // global error template
    error: errorTpl
});

// somewhere in your app
ATV.Navigation.showError({data: {title: 'Error', message: 'This modal can be dismissed'}});

Let me know if this works 🙌

@dnicolson
Copy link
Author

Here is a fork showing the modal: https://github.com/dnicolson/appletv-demo

I tried using a global template but the button didn't dismiss the modal either.

@emadalam
Copy link
Owner

emadalam commented Feb 2, 2020

I see a couple of issues with the usages.

  • You are calling showError right inside of the ready function. This isn't the indented usage. You are supposed to call either resolve or reject with the data that gets passed to the page or error template respectively.
  • The way alert or error documents are pushed on to the stack is, if there is no document that exist in the navigation stack, it gets pushed as the first document on the stack, in which case you can't really dismiss the window. In your code, the race condition might cause the error document to be the first document on the stack and currently shown to the user, and thus cannot be dismissed.
  • The reason why the second example from your code works, is because you are manually handling the document creation and presenting the modal, so atvjs framework has no control or clue about your code.

Having said that, what is the use case that you are trying to solve? If you are just trying to show an error on some failure during page load, I recommend you use the reject callback with whatever data you want to pass to the some global custom error template. Also make sure it's not the first page that your user interacts with that generates the error, in such a case, you cannot dismiss the only thing available on the stack.

const HomePage = ATV.Page.create({
  name: 'home',
  template: template,
  ready(options, resolve, reject) {
    // some async action for success
    // resolve({data})

    // some action that results in error
    reject({
      status: 'CUSTOM_ERROR',
      data: {
        // custom data
      }
    })
  }
});

const customErrorTpl = (data) => `
<document>
  <descriptiveAlertTemplate>
    <title>${data.title}</title>
    <description>${data.message}</description>
    <button data-alert-dissmiss="true">
      <text>Ok</text>
    </button>
  </descriptiveAlertTemplate>
</document>`;

ATV.start({
  menu: {
    items: [{
      // make this static page your homepage that will not generate errors
      // the user will be navigated to this page on app load without errors
      // as such the navigation stack will not be empty for subsequent
      // navigations and any subsequent errors can be dismissed
      id: 'search',
      name: 'Search',
      page: SearchPage,
      attributes: {
        autoHighlight: true
      }
    }, {
      id: 'homepage',
      name: 'Home',
      page: HomePage,
    }]
  },
  templates: {
    // custom status based error template
    status: {
      'CUSTOM_ERROR': customErrorTpl
    }
  },
  onLaunch(options) {
    // navigate to menu page
    ATV.Navigation.navigateToMenuPage();
  }
});

Please mention your intended use case that you are trying to solve so that I can help you further. This doesn't seem to be a bug, it's rather a framework usage and understanding issue tbh.

@dnicolson
Copy link
Author

Thanks for the prompt and detailed follow up.

  • The reason why the second example from your code works, is because you are manually handling the document creation and presenting the modal, so atvjs framework has no control or clue about your code.

I'm curious as to why the modal is not dismissed as the ATV code looks quite similar to the navigationDocument.presentModal() code and navigationDocument.dismissModal() is called when the button is used.

I have updated the fork so it calls reject in favour of creating a new modal. I'm experiencing an unhandled promise rejection though when using the "Play" button in the demo app.

I suspect it has something to do with the PlayPage being invoked with a data-href attribute, but this comment seems to suggest that it is possible.

@emadalam
Copy link
Owner

emadalam commented Feb 2, 2020

I'm curious as to why the modal is not dismissed as the ATV code looks quite similar to the navigationDocument.presentModal() code and navigationDocument.dismissModal() is called when the button is used.

There are a lot of anomalies and weird behaviours that can cause your app to crash if you use the raw TVMLKit JS. E.g if you try to pop the last element from the default navigation stack, you'd notice an app crash. As such this thin layered atvjs framework exist to let developers focus on app creation without the worry of such anomalies. So the APIs might look similar but under the hood, atvjs takes care of these anomalies by maintaining internal states where needed and extending the default behaviours, provided you use the atvjs' methods instead of the raw TVMLKit JS methods.

I have updated the fork so it calls reject in favour of creating a new modal. I'm experiencing an unhandled promise rejection though when using the "Play" button in the demo app.

I don't really understand what are you trying to do, the code in your fork doesn't really make much sense. Are you familiar with the async and promise concepts and in general the general JS + Web concepts? If not, it'd be great to get a familiarity with the basic concepts before diving straight into the app usage and not try just trial and error.

I don't see you followed the sample code that I wrote here, please read through the code to see the proposed changes. The code clearly said to add a status base CUSTOM_ERROR template and then use the same status while rejecting the promise. And in your fork, instead of initializing the app with status based custom error templates and later using reject with the same status, you are just trying to randomly call reject with a random string value here. Of course that's an unhandled rejection. It's the same like adding a random throw new Error() statement somewhere in the code and then wondering why is there an error thrown 🤷‍♂

I'm afraid I'm not able to help you any further until you have tried the steps shown in the code above 🙏

@dnicolson
Copy link
Author

Sorry for the confusion, the examples in the fork were contrived and for the sake of brevity only included minimal changes from the original code.

I was following the appletv-demo example where a promise is rejected without a status from a page invoked with a data-href attribute (reject({status: 500}) works though).

I have updated the code which shows that from the HomePage all promise rejections are caught regardless of the status and the global error template is shown with the correct data. Promises rejected from the PlayPage need the correct status and are caught but the data object is undefined and the modal cannot be dismissed with the Ok button.

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 a pull request may close this issue.

2 participants