Android_UI:Theme Selector and related fixes

[Zer0xFF]

Android:Set Column Width Needed For autocolumn to work: if you look at this one, you'd notice I've only set gridview column width for isConfigured the reason is, we are using a fixed dp for covers as such i was comfortable using that value. but for "if (){} else {//this}" I didn't set the size as 'else' would set the column size for the initial file picker... however without setting an actual width, only 2 column would ever appear, irrespective to screen space. as such, I'd recommend you try and remove the if statement as see if gameGrid.setColumnWidth((int) getResources().getDimension(R.dimen.cover_width)); itself is good enough to be used for the initial file picker as well.

Android:Remove TableLayout, Fxed Padding, Small Clean ups. Android:Fix:Fixed Padding Android:Removed LinerLayout (redundant) should be all one commit, but i only realised the mistake after committing and the last one was a result of rebase, since tablelayout was changed, it wasnt removed by the initial commit. anyways, just so, the padding issue, didn't exits before the merger(that enabled rotation), since screen orientation was fixed, but now everytime the screen is turned, it would cause more padding to be added causing it to build up, as such i'm using the relative_layout_original_right_padding to get the original padding, so i can reset the padding according to orientation each time. as for removing TableLayout/LinerLayout, now we're using the gridview for both the initial file picker and cover display (game picker), thus it's not needed.

I know this pull is part fixes and part a new feature, but since they're all ui as such somewhat interconnected, there was little point in splitting them. again, you can always cherry-pick things out if you dont want them.

Android:Fixed Padding: fixes cover disappearing before the actionbar and also adds bottom padding, so that bottom covers are not be at the very edge of the screen.

Android: this function would get called by onNavigationDrawerItemSelection as the name suggests, the function removed is actually called twice, once on NavDrawer creation and once on Main View creation, as such i removed the mainview one, since the NavDrawer one is linked to the working of the Nav itself.

@LoungeKatt commit, last just moves the settings UI around. Android: Restore Strings (Merger Conflict): restore missing string due to commit above it.

Sorry @jpd002 for adding more to this pull, its just most of the addded changes are small (fixes) and should be easy to review, just so you probably want a stable enough build before you can do the weekly and those fixes should be as named, fixes, and shouldn't be breaking anything anyway

Android:Fix ELF using other games cover due to the way adaptors work, they recycle off screen views instead of creating more to fill the screen, in this case since we're only setting covers when they exist and we dont explicitly unset them when the don't, if a reused view already contains a cover, when an ELF is being displayed it will keep that cover, this fixes that issue.

[Zer0xFF]

question, should we give user control over top and bottom gradient colour or just the bottom?(if any) giving them control of the top could throw off the theme's consistency, while not so much with the bottom one. (but that's also personal perference, but if we do give them control of the top, we should probably give them control of nav drawer and action bar... especially since they're linked) just as well, I could give them full colour control, or maybe a toggle between white and black?

bottom gradient is Color.rgb(20, 20, 20)(default) bottom gradient is Color.rgb(235, 0, 0) bottom gradient is Color.rgb(235, 235, 235)

[AbandonedCart]

It might help to change the heading "UI Settings" to "Theme Settings". Most of the general settings are UI settings as well, such as virtual gamepads and FPS display. Even the image cache and folder could be considered UI, but none of them relate to the theme.

[Zer0xFF]

Ya I was thinking about that. But I was contemplating if I should break it down to emulator ui and app ui or have itball under ui and inside they can have sub heading... As such I left it like this.

[AbandonedCart]

Do what you want, but this was how I did it: https://github.com/AbandonedCart/Play-/commit/a6d9db96828650c48ef47ff6da4505fb590a0f8d

[Zer0xFF]

@LoungeKatt ya this works. cherry-picked, thanks, I'll push it out a bit later.

I've also kept the debug string, as they'll mostly likely be added at later stage anyway right? or should i remove them?

[AbandonedCart]

@Thunder07 It wouldn't hurt to leave them. They can always be removed later.

[AbandonedCart]

@jpd002 @Thunder07 Those lines were the ones from the old drawer. Somehow that ended up backwards in my fork. I had updated the new ones to past tense (for use with Accessibility) but those ended up back in there. It should be <string name="navigation_drawer_open">Navigation drawer open</string> <string name="navigation_drawer_close">Navigation drawer closed</string>

BTW with 16f6c3e0ada8a1d5d31a95cdd04a2c35cebc75ec, you no longer have an initial load of the list unless that moved to another file.

[AbandonedCart]

I just noticed the first paragraph.

if (isConfigured) {
        gameGrid.setNumColumns(GridView.AUTO_FIT);
        gameGrid.setColumnWidth((int) getResources().getDimension(R.dimen.cover_width));
} else {
        gameGrid.setNumColumns(1);
}

Should fix that issue.

[Zer0xFF]

@LoungeKatt The solution you wrote to the 'issue' will force one column even on Android TV. (edit2:maybe i wasn't clear enough, I don't want it to be just 2 column, since it'll be a waste of space, having it just 1 column is even more so) I merely suggested that I didn't know which size to use as such I used the if statements. (edit: removing the if statements and using cover width as default ("gameGrid.setColumnWidth((int) getResources().getDimension(R.dimen.cover_width));") is my suggestion(i'm just not sure if its the right size, although it seems to be working nicely on my phone))

Also that line I removed is called twice once by onCreate and again by by the navigation drawer setup, so there will be an initial load. (as it stands, it's called twice)

[AbandonedCart]

@Thunder07 I couldn't find the drawer setup call, but I guess I'll take your word for it.

With the initial file selection, is there a reason for multiple columns? If it looks identical to the game cover grid, that confuses people (<- All the comments saying "my covers never showed up" because they quit before clicking one). The way it looks now with two columns and the text wrapping in a box next to an icon looks sloppy.

It's a fair point it may look different on Android TV, but there is also the isAndroidTV check to make that 2 columns if 1 is wasted space on that size.

[Zer0xFF]

@LoungeKatt https://github.com/jpd002/Play-/blob/master/Source/ui_android/java/com/virtualapplications/play/NavigationDrawerFragment.java#L82

while I might be looking a bit too much into the future, once I'm done with the indexing code, their wont be a need for the initial file selection.

but even still, it doesn’t quite look the same, it's still using file list layout, except now it's multicolumn, again, I just think its a waste of space, especially on tablets, TV and large devices.

also, if you thing people are will get confused... well, I don't see why they wont get confused anyway. surely, the initial view, as the name suggests is initial, they have no clue what comes after it, if they should be expecting 1 or 2 or 100 columns.

[AbandonedCart]

That is a question for @LinkofHyrule89 could better answer. It was his initial request for making them look different that led to the single column file selector.

Disregarding the theme, this is how the columns look on a phone:

I'm not saying anything is wrong with it, but since you mentioned wasted space and I see cramped, it seems like we might be seeing it from two different device types (phone : tv / tablet)

[AbandonedCart]

It could also be the fact that all of my design classes made the point that text that appears

In a box that bre aks it a part

should only be done when it was an intentional style choice. Eh, whatever you wanna do.

BTW thanks for pointing out that line. It makes more sense now because it was the line you pointed to that I removed to resolve the same issue (and forgot :/). I was looking at it that Android TV wouldn't be using the navbar so it shouldn't rely on that to load the list.

[Zer0xFF]

Ya, turn it around with only 1 or 2 column and you'd see what I mean. I'm not saying it doesn't work, but it can be a bit better (again, this is mostly design preference, so we can go at it all day, I rather not) But one last thing, since you posted the screen shot, I didn't consider vertical space as well, but that's solvable by centering the gridview.

[AbandonedCart]

Fair enough. Like I said, it's how I was taught. As long as you've seen it from both angles, then it is a choice (which falls under "should only be done when it was an intentional style choice").

The vertical space is a good thing. Centering it will likely look odd for a file list. As icons and text, I completely agree. In this context, weird. For the covers, though, that makes sense.

[jpd002]

Tested on a tablet, phone and TV and everything seems to be working good. So, I'm merging this. Thanks!

[LinkofHyrule89]

Great work guys!

[Zer0xFF]

Cool :)

[Zer0xFF]

@jpd002
Since removing the prepareFileListView() from onCreate(), on the very 1st initial run, you'd get the gridview to display but with no covers or description but with a name and placeholder cover, since prepareFileListView() is called before DB import... this can be changed back to normal behaviour by moving the DB import before setContentView*... however, even with this behaviour, after the DB import is completed, the gridview will be updated with covers.

*however, if you do that,'fix it', you'd get the loading dialogue which will prevent the user from accessing the UI until the import is done... personally, I prefer the current behaviour, since the user can start a new game, while the details are been downloaded in the background. if you believe this behaviour might be confusing, you can just move DB import call as i said earlier.

[AbandonedCart]

Or Revert https://github.com/Thunder07/Play-/commit/16f6c3e0ada8a1d5d31a95cdd04a2c35cebc75ec and replace the contents of the else in https://github.com/jpd002/Play-/blob/master/Source/ui_android/java/com/virtualapplications/play/NavigationDrawerFragment.java#L80 with

    int user_position = Integer.valueOf(sp.getString("page", "0"));
    if (mCurrentSelectedPosition != user_position) {
        selectItem(user_position);
    }

(Corrected to apply to categories, not fragments)

[Zer0xFF]

now I'm confused, why would you do wanna do that? as it stand it will always return 0 now, but I plan on adding an option to allow the user to choose their own default sorting... at which case it may not be zero and prepareFileListView() would be called twice in such case. so the other solution would be to remove that selectitem all together... which brings us back full circle... as moving the import to before the view is called, will have the same net effect as removing the selectitem.

also, how do you mean it will be role of the dice, the import DB call seems to prevent new asyncs from starting, so if its moved to the top, the async that set up the gridview will be blocked until the import is completed and vice versa.

[AbandonedCart]

I removed it locally, but for some reason I was thinking of fragments looking at pages.

Considering they are just sorting methods, then removing it altogether does make more sense. The navbar doesn't need to load a preference for the MainActivity. Besides, the navbar is only for phones.

As for the rest, it applies to both methods.

[Zer0xFF]

you arent wrong, that was their purpose, but we have no fragments here, so it will be used for sorting.

[AbandonedCart]

Ok, so getting a variable from the navbar whose only real purpose it to pass to the MainActivity, or even worse, reloading the entire view to load an integer preference. Long as it makes sense to you, I guess.