Rework themes and fix overflow icon color

[Robyer]

I reworked themes, especially setting Toolbar's theme and popupTheme which was broken (it showed overflow icon in wrong color).

Also it's not needed to define own arrow drawables or set that toolbar colors programatically, we can rely on the AppCompat.

Note this PR breaks compatibility as I renamed themes and attributes.

As I tested this on demo app it should work correctly, but please check it too :-)

Btw I wasn't sure about naming the themes, but previously it felt redundant and messy, so I kept only:

• Theme.Mal.Light (with dark toolbar)
• Theme.Mal.Dark (with dark toolbar)
• Theme.Mal.Light.LightActionBar (with light toolbar)
• Theme.Mal.Dark.LightActionBar (with light toolbar)

Overflow menu has same color as the base theme (so Mal.Light has light overflow menu and Mal.Dark has dark).

[Robyer]

@daniel-stoneuk Btw as I added "refresh" item to overflow menu, you can test one bug I found. When you scroll a little bit down (e.g. to hide the title item, but still see the Version item) and press the Refresh, it will scroll up. But when you scroll down much more (e.g. completely to the bottom) and then press the Refresh, it won't scroll anywhere.

I believe not scrolling anywhere (= keep same scroll position) after refreshing the list is correct, but I'm not sure what causes the problem.

[daniel-stoneuk]

Hey @Robyer, thanks for the PR looks like you've made some good changes to fix some of the short sighted decisions I had made. I'm impressed that you've got it all working without having to programmatically set button colours - to be honest it was quite a hacky workaround back when I added themes.

Not really sure why you would refresh the list, perhaps in case you wanted to add an item?

Anyway, it looks like there are no unwanted side effects of the PR so I will finish testing it out and then merge and create a new release. Just to speed things up a bit I would greatly appreciate a short list of breaking changes that you made. Thanks!

[daniel-stoneuk]

Regarding the refresh scrolling issue, it looks like it's trying to scroll to the top of the first item if it's currently near the top. Weird but not a major issue however do let me know if it's something we should definitely look into.

[daniel-stoneuk]

@Robyer Check out the release and let me know if there's anything else to add. Thanks very much for contributing.

[Robyer]

@daniel-stoneuk I'm always impressed with the fast speed of your responses :-)

Not really sure why you would refresh the list, perhaps in case you wanted to add an item?

You need to refresh the list for example when you have some dynamic items - as the last item in Demo app. If the item would be near the top, each click on it would trigger the scroll to the top.

EDIT: for example I experienced it when I had dynamic item that I updated in onResume(), which means after going to another activity and back, whole list scrolled to the top.

Just to speed things up a bit I would greatly appreciate a short list of breaking changes that you made.

Breaking changes are:

• Removed mal_lightActionBar, mal_popupOverlay attributes. Added mal_toolbarTheme, mal_toolbarPopupTheme instead.
• Renamed the themes

[Robyer]

Also, docs should be updated. I saw at least this part:

Ensure that the theme extends either of these themes, and apply primary & accent colours: Theme.Mal.Light.DarkActionBar Theme.Mal.Light.LightActionBar Theme.Mal.Dark.LightActionBar Theme.Mal.Dark.DarkActionBar

[daniel-stoneuk]

Thanks, you've thought of everything! Heading to sleep now so will finish anything off tomorrow. Have a great evening :)

[Robyer]

I've updated some stuff in readme.

Also in release notes you can mention (it's not related to the PR, but my previous commit in repo), that it's not longer needed to catch NameNotFoundException when calling ConvenienceBuilder.createVersionActionItem().

[Robyer]

Oh, and as I was just looking at the theme names... and I think that chosing:

Theme.Mal.Dark (everything dark, including actionbar)
Theme.Mal.Dark.LightActionBar (dark, but light actionbar)
Theme.Mal.Light (everything light, including actionbar)
Theme.Mal.Light.DarkActionBar (light, but dark actionbar)

would be wiser / look better / more intuitive? than my PR way:

> Theme.Mal.Dark (everything dark, including actionbar)
> Theme.Mal.Dark.LightActionBar (dark, but light actionbar)
> Theme.Mal.Light (light, but dark actionbar)
> Theme.Mal.Light.LightActionBar (everything light, including actionbar)

But I don't know, maybe I am overthinking it :-)

[daniel-stoneuk]

I think how you've done it is perfectly fine! How does AppCompat do it's theming, I'm just updating AS right now so can't check.

[daniel-stoneuk]

Just checked and yeah you're right it would probably be more intuitive. Should we change it then or leave it?

[Robyer]

I vote for changing it, if it isn't big problem for you.

[daniel-stoneuk]

Yep I'll do it now!