-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add about #18
Add about #18
Conversation
Hi, thanks for the PR. I have another idea, how this would be solved better: I wouldn't look for the "about" information in the settings. Instead of showing the three fixed icons in the top right, add a menu to the action bar. In <item
android:id="@+id/menu_about"
android:icon="@drawable/ic_TODO"
android:showAsAction="never"
android:title="@string/btn_about"/> When the user taps on this menu entry, a dedicated about activity is shown. The license information should also be part of that about screen. What do you think? Would you like to implement this? |
I am happy to see a menu. I do not know when I will get to this or who will. The code is quite modular and it can be moved. Personally, I would merge it and create a new issue to create a menu - this change is better than before but not perfect - in my eyes. It might be that this fix will not be shipped for a while if it is not merged. But that is also your decision as a maintainer. My maintenance style would be hopefully "Done is better than perfect" & "small steps with fast feedback" and for community: "engaged and merged (and reverted) is better than rejected/stale" - but they have to be weighted with long-term maintainability - so your choice as the maintainer! I am too tired to do it now properly. |
- Version information - License - Repository URL and button to visit it
There are a few issues with this PR:
A lot of this is due to the approach of putting content into the settings view, which isn't made for that. This results in things like having to write a custom layout, just to wrap the content. As for the button, I'm not sure if the approach of just putting the URL into an intent is valid at all. It probably depends on the Android version, whether this works or not. A much easier approach is simply making the URL in the text view clickable. I rebased your branch on top of current |
- Button doesn't seem to work on API 31, remove it and replace it with linked URL instead - Fix a few layout bugs - Remove unused imports
This fixes #16.