search

primaeval / script.tvguide.fullscreen

tv guide fullscreen
GNU General Public License v2.0
25 stars 26 forks source link

skin tweeks #60

Closed im85288 closed 8 years ago

primaeval commented 8 years ago

I can't add that in until you leave space for at least a dozen categories and extra buttons for context menus, quit etc. One of the criteria for this is to make it usuable on touch devices, even phones so all the functionality has to be touchable eventually. Another criteria is everything has to be Fullscreen to include maximum data density on small devices. There is still a fair bit of functionality to add to the menu.

2 video windows at the same time is too much for low end devices like my rpi and phone.

primaeval commented 8 years ago

I've started a wiki page with Design Criteria just so we are clear on the goals of this project. https://github.com/primaeval/script.tvguide.fullscreen/wiki

im85288 commented 8 years ago

What's the problem here? It took a lot of time tweeking things how I like it, the menu dialog works well particularly with the context in quick EPG. This is becoming frustrating to be honest. It's not he default skin so I don't see an issue. Can you elaborate

primaeval commented 8 years ago

I appreciate that you have put a lot of work into this but I have put months of time in that I really didn't intend at the beginning. A lot of people have asked for features I just didn't want or need but I try to be friendly and add them in if possible if it could benefit everyone.

The main thing that is a problem in this case is the lack of space for an expanded category list. The first person I added lots of changes for had a requirement for a large list of categories to sort a huge number of channels: eg countries, sports, entertainment etc.

I also mentioned that there are going to be more buttons in the menu at some point.

The double video window will crash low end systems.

You really have to think of the impact on the whole system when you add changes. If you want to hack it exactly as you like it then that is a fork not a team project. This is open source so you can do what you want but for the project I am presenting to users it has to have the same core functionality in every skin that is included.

im85288 commented 8 years ago

The categories can be scrolled so as others appear, I did the same with the other buttons (so in fact with this PR there are more buttons available to choose from - though not immediately visible). I don't have a touch device, are you saying they cannot be scrolled on touch devices?

I'm trying to avoid going my own fork but if that's what's needed I'll do so and send in a PR to remove the sly skin.

primaeval commented 8 years ago

That would be really tricky to scroll on touch devices. You need to add a scrollbar too when there is more than one scrollable list on a page with a linked id. Have a look in some skin files to see how its done.

I don't think we are going in too different directions but please try to make sure your changes can run everywhere within reason and don't make me spend days trying to fix them. I am getting a bit touchy after the Up Next debugging that took most of last week.

im85288 commented 8 years ago

I think it's safe to take the Up Next out or at least disable it in the settings by default. I understand what you are saying with making things work on all devices but as I do not use touch devices I cannot test it there and found the menu setup screen as it was prior to my changes to waste too much white space. Additionally as I mentioned briefly above having the context menu not show full screen works very well when you access the context settings from the quick epg.

As it stands now I really do not want to add scrollbars in that view as I feel it messes with the minimalistic view I was aiming for there.

Let me know how you want to proceed as I do of course want to continue contributing here as much as possible but if there are too many compromises with changes I make to my own skin then it becomes a bit self defeating.

primaeval commented 8 years ago

Ok. I'll have a test later but can you make sure there is only one video window. When you are sure it doesn't affect any other skins submit another pull request. I blindly added it then tested it this morning. Normally I use the command line and make a test branch.

You don't have to make the scrollbar visible. It just enables the scrolling to work with more that one list control. I came across that problem in my skin.naked.

im85288 commented 8 years ago

I re-added the changes in this PR: https://github.com/primaeval/script.tvguide.fullscreen/pull/64 and assured there is only one video window active at a time.

I am and was sure when I made the original PR that it does not effect any other skins as the changes are solely for the sly skin (with the only exception being the inclusion of having a context menu on the quick epg)

For future ones please do not merge then take the changes back out as it makes the whole re-merging mechanism painful, better to just not merge in the first place.