-
-
Notifications
You must be signed in to change notification settings - Fork 81
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
Enhancment/update theme fragment #195
base: develop
Are you sure you want to change the base?
Enhancment/update theme fragment #195
Conversation
…/peakvalleytech/Presently into enhancment/update-theme-fragment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this! I need to think about the UI for a bit before we can land this
@@ -1,81 +1,102 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android" | |||
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this LinearLayout
needed for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alisonthemonster Hey, sorry for the late reply. It's used to create the border. The easiest implementation I know currently.
android:layout_marginBottom="8dp" | ||
app:layout_constraintBottom_toBottomOf="parent" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:srcCompat="@android:drawable/checkbox_on_background" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in order to merge this PR we'll need to find a different icon that matches the Presently style. I don't have the capacity right now to design something so this might have to wait a bit!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I thought so too. I had to get an icon on there for demonstration. We can change it once we find or design a suitable one. I'll leave that to you.
Just let me know when you get one. Really feel like this would be a huge and valuable addition to the next release.
@alisonthemonster So, have you had the chance to think about how to proceed? I really believe this is the best UX improvement that we can do currently. And it is already done! If you have time please think it over so we can get this merged. If you need any more changes let me know. |
@alisonthemonster Have you had any spare time to make the icons? I really believe this would add a tremendous amount of user satisfaction. |
📜 Description
Fixes #188, theme selection page should match the current theme.
I'v change the background to match theme. And to adress the problem of some themes blending with background, I add a white neutral border. You can change it to another color as you see fit, but I think the white looks best.
Also i added a icon to indicate the current selected theme. I think it improves the UX dramtically.
I think the icon is ok for now, but should propbably be changed in later versions.
💚 How did you test it?
Manuually tested UI in relevant android versions.
📸 Screenshots / GIFs