Feature #221
closed
thanks for the feedback - had a brief look. a few issues:
submitted patches need to be split rather than all in one.
the patch contains tab indentations, mixed crlf line endings, and new files should be included in the diff.
it might be easier if I re-implement this, as due to the size of your patch it will be hard to check everything, and go from there. I also may have some of this patch already part implemented on my working copy as I had already looked at the action changes some weeks ago.
quick question: when you say some other changes are needed for it to run correctly, could you give me more information - you mean the code applies without the other changes but the feature doesnt work ? I dont see why we would need to lose the buttonscroller control for example. I'm happy to remove that, but only when I have confirmed none of the old skins use it, as I dont want to break an older skin that some people might still be using.
since you'll be re-doing the whole thing, it's irrelevant to discuss what changes are done to make this work from my perspective; by doing so, i may just confuse you more than i would help.
as for the buttonscroller, i simply found it easier to remove it rather than to change the code to handle the actions as the rest of the controls would since this changes were not implemented on main xbmc.
anyway, i tried but seems c++ is not my friend... thanks for looking into it.
I was just interested in case I had missed something.
Your help is appreciated and I don't want to put you off from contributing. the issues are minor and mostly formatting etc.
i have been working towards this btw. turns out there is a lot of other stuff related to the nav back changes. the conditional stuff could be done without it with some changes, but I thought to pull in some other bits. Just wanted to comment so you don't think I had forgotten ;-)
thanks BuZz... the objective, however, was not the NAV_BACK but the conditional OnClick portion. The only reason I made the changes pertaining to the NAV_BACK was because I didn't want to spend too much time figuring out just the conditional OnClick stuff. I worked for me the last time i made the build.
Either way, thank you kindly!
yeh no worries - I know the objective. I was just saying that I decided to look into the nav back stuff on the way there :)
Phew finally - added in r31666. Please give it a test and let me know how you get on.
notes: As you pointed out some previous patches were required for it to apply more cleanly. However the ACTION_NAV_BACK was part of a larger set - see r31662 (xbmc https://github.com/xbmc/xbmc/pull/253). I also added skin support for "onback" see r31664 (xbmc https://github.com/xbmc/xbmc/pull/336). Prior to this, I did some other changes such as splitting our keymap xml and some other stuff the same as mainline has so I could more clearly verify the changes needed for r31622.
- % Done changed from 0 to 100
- Estimated time deleted (
1.00 h)
- Resolution set to fixed
Marking as fixed - but please do pass any feedback back.
- Status changed from New to Closed
Also available in: Atom
PDF