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

Support menu item reload configuration #15

Closed
emadalam opened this issue Sep 15, 2016 · 13 comments
Closed

Support menu item reload configuration #15

emadalam opened this issue Sep 15, 2016 · 13 comments
Assignees

Comments

@emadalam
Copy link
Owner

Currently

Selecting the menu item loads the previously loaded cached page and there is no way to force reload it based on configuration.

Wanted

Support attribute configuration reloadOnSelect to load a non-cached version of the page on every selection.

...
menu: {
    items: [{
        ...
        attributes: {
            reloadOnSelect: true
        }
    }]
}
...
@emadalam emadalam self-assigned this Sep 15, 2016
@cpwc
Copy link

cpwc commented Dec 28, 2016

Is there a workaround to reload the pages for now?

@a-ignatov-parc
Copy link

@cpwc I think you can manually update page document related to menu item using setDocument method. Also you will have to create new page instance using create method.

If you need to reload page when menu item is selected you can use TVML events. It would be something like this:

// Usually menu is rendered as first document so we retrieving it from global documents list and adding event listener to be able to update page on menu item select
navigationDocument.documents[0].addEventListener('select', event => {
    // Updating page only when `menuItem` is selected and page id is equal to what we are looking for
    if (event.target.tagName === 'menuItem' && event.target.id === 'my-target-page-id') {
        // Getting rendered page config from attached page document
        const menuBar = event.target.parentNode;
        const feature = menuBar.getFeature('MenuBarDocument');
        const prevPageDocument = feature.getDocument(event.target);
        const prevPageConfig = prevPageDocument.page;

        // Creating new page instance and updating menu cache
        ATV.Menu.setDocument(ATV.Page.create(prevPageConfig), event.target.id);
    }
});

I haven't tested it but I hope you get the idea ;)

P.S. atvjs is great lib but I think everyone can agree that it has too many dependencies and restrictions. I've tried to write my first app using it but came to creating set of tools that give you power of incremental updates using virtual dom and more declarative approach in creating apps using TVML. If you are interested go and check it here ;)

@emadalam
Copy link
Owner Author

emadalam commented Dec 28, 2016

@cpwc You may use the Milestone-Enhancements branch for now until it gets merged to master. I have added this functionality and you will need to add the configuration reloadOnSelect as mentioned in the issue description. Let me know if that works for you..

...
menu: {
    items: [{
        ...
        attributes: {
            reloadOnSelect: true
        }
    }]
}
...

@a-ignatov-parc Thanks for your inputs, your solution will also work 👍 Now coming to the two main things that you pointed out, too many 1) dependencies and 2) restrictions. I would really appreciate if you could elaborate on those points, haven't had a chance to get an extra pair of eyes, would be really helpful if you could tell what dependencies and restrictions you precisely see so as I can improve upon those. Or if you prefer suggestions, feel free to open issues, or even contribute.

On a side note, the whole purpose of this library was to be minimalistic and provide a rock solid architecture to quickly build your TVML applications. I have recently added a demo app with streaming examples https://github.com/emadalam/appletv-demo and also the boilerplate code https://github.com/emadalam/appletv-boilerplate. I think there is a need to add more documentation and sample examples to highlight the many possibilities available with atvjs which are still not explained or demoed anywhere as of now.

P.S. @a-ignatov-parc I have checked your TVDML library before as well, nice work 👍 People who like that declarative approach, would really like TVDML.

@a-ignatov-parc
Copy link

a-ignatov-parc commented Dec 28, 2016

@emadalam sure, let me try.

Dependencies:

  1. You are using babel-polyfill that isn't much needed but have big performance and size footprint. And not to mention that only Promises are used from this set of polyfills.
  2. You are using lodash for things that could be easily implemented in ES6. I think that user should decide what tools and template engines he want to use (if we are talking about string templates). Documents in TVML are very simple and short, you don't really need something as powerful as handlebars. ES6 string templates as default template engine would be enough ;)
  3. Same thing with pubsub-js. It's included but not used at all.

That is why atvjs is really big! 155 KB for minified version.

Restrictions:

  1. There is no ability (correct me if I'm wrong) to update rendered documents in runtime based on events or something else. Developers have full power of DOM but no lifecycle hooks to use it. Found undocumented method afterReady that solves that restriction :)

@a-ignatov-parc
Copy link

Also using string templates restricts how you update rendered documents. You can't use such templates to flush updated state because you will loose user's focus. The only solution is to manually update nodes in afterReady hook. And when you do this you are ending with non deterministic rendering that is hard to predict and test.

@cpwc
Copy link

cpwc commented Dec 29, 2016

@a-ignatov-parc thanks for that snippet it works well for my current use case to reload the page.

@emadalam I will test out your current branch! Thanks for getting it out! It really helps a lot when contents under menu page requires updating. 👍

Edit: Works great!

@emadalam
Copy link
Owner Author

emadalam commented Jan 1, 2017

@a-ignatov-parc Thanks for the inputs, much appreciate 👍 Let me try to answer you with contexts.

  1. I had written atvjs at the time when AppleTV platform was still in it's early stages and was buggy. The ES6 syntax and many other things used to break back then (which have been patched and fixed over the year by Apple). As such babel-polyfill was included to cater for anomalies, a stable working code outweighed the performance and size footprint which is negligible. The AppleTV device is fast enough and I have used atvjs in large scale streaming applications comparable to the likes of Netflix and Hulu without any noticeable performance lags.

  2. I agree that the use of lodash is a bit of an overkill, however as mentioned earlier, the platform was buggy and unpredictable back then and I chose stability and faster development time as opposed to breaking the head over trivial issues. And regarding template engine, handlebars is not a dependency for atvjs. However for the code samples, I used it because I feel handlebars is very handy when it comes to handling large scale applications, especially the template helpers, can give your code a proper structure with complete separation of concerns.

  3. pubsub-js was added to act like a global event bus for your application. I should have included examples and sample code to make people understand how to leverage that. Now you might argue why use event based system when the whole front-end world is moving towards state based architecture. I would say in case of how tvml apps work, event based architecture is best suited. To give you the context, imagine you have navigated to Movie details page from the homepage and you want to favourite a movie by clicking a button. And let's say you have a feature to list out your favourite movies in the homepage. But since the way tvml apps work, the moment you press back button, you'd be on the previous page without reload of the page as it's just lying on the navigation stack. How'd you ensure the DOM lying somewhere on the stack gets updated by some action on a totally unrelated page somewhere else on the stack. A state based system would mean you would have to manipulate the whole stack, not when using an event bus 🚌

Having said that, yes I agree that now the AppleTV platform seems stable, things like babel-polyfill, lodash etc can be stripped down. I would add that as an enhancement issue and would definitely work towards resolving it. But given the fact that there isn't much contribution from anyone from the community and me not owning an actual device, testing and releasing a stable build is a challenge and would require more time. Nevertheless, I'd appreciate contributions in any form, be it even bugs or ideas documented in issues. Thanks 🙏

@emadalam
Copy link
Owner Author

emadalam commented Jan 1, 2017

@cpwc I'm glad it solves your specific use case. Feel free to open issues and feature requests, any helping hand is much appreciated 🙏

@slavixxx
Copy link

Where I can see other menu attributes and options for menu?

@MarhyCZ
Copy link
Contributor

MarhyCZ commented Jul 6, 2018

Hi,
are you planning to add "reloadOnSelect" into mainline branch? I have no problem with including it by myself from your great commit in Milestone enhancements, but I think it's a really nice feature, that would come handy for a lot of people that may not notice that it exists.

@emadalam
Copy link
Owner Author

@MarhyCZ, it would be great if you can submit a PR to this, including the update to the readme file mentioning this support 🙏

And do mail me if you would like to get involved with the project as a main contributor, I'm really short of hands and time to take care of everything here 🙅‍♂️

@MarhyCZ
Copy link
Contributor

MarhyCZ commented Aug 2, 2018

@emadalam I will have it submitted by today! Sorry for delay, holidays :)

I really appreciate this offer, but I don´t consider myself as a programmer, I am just basically merging the branches :-D There is no rush in anything, I think the current version is great for a lot of people who want to create content-driven tvOS apps 👍

@emadalam
Copy link
Owner Author

The actual ticket issue has been merged now in the main branch and released as part of #44. Closing this for now.

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

No branches or pull requests

5 participants