minor changes to toolbar style and other UI elements #2529
Conversation
|
Hi @dkanada , Thanks for this new PR. Look good to me. You may be interested by doing what's in RiotAppCompatActivity: Benoît |
|
Hi @dkanada , Can please fix lint error reported by Travis please? Thanks you |
dkanada
changed the title
|
Signed-off-by: Dillon Kanada [email protected] |
|
I think it also fixes #2305 👍 |
|
Hi @dkanada , Thanks for the update. LGTM, I'll try your change tomorrow. Benoît |
| videoView.setOnPreparedListener(new MediaPlayer.OnPreparedListener() { | ||
| @Override | ||
| public void onPrepared(MediaPlayer mp) { | ||
| mp.setLooping(true); |
There was a problem hiding this comment.
So the video is played forever, I'm not sure it is a good idea. Can you revert this change please?
There was a problem hiding this comment.
I reverted this, but I actually think it is a great idea. Probably 90% of my videos are used as gifs which benefit from a looping format, and if they are long enough to be actual videos then a single loop every minute or so will not be too jarring of a user experience.
There was a problem hiding this comment.
I understand this point of view, but maybe open a separate PR to introduce this change. In this PR it looks a bit offtopic :-)
| </RelativeLayout> | ||
|
|
||
|
|
||
| <im.vector.view.PieFractionView | ||
| android:id="@+id/media_slider_piechart" | ||
| android:layout_centerHorizontal="true" | ||
| android:layout_centerVertical="true" | ||
| android:alpha="0.4" |
There was a problem hiding this comment.
I prefer to keep the alpha to 0.4, it looks better. Can you revert this change please?
| @@ -1,2 +1,25 @@ | |||
| <resources> | |||
| <style name="AppTheme.Light" parent="@style/AppTheme.Base.Light"> | |||
| <item name="android:navigationBarColor">?attr/primary_dark_color</item> | |||
There was a problem hiding this comment.
The vast majority of the biggest Android applications does not customize the navigationBarColor (at least for the entire application). It can be useful for instance for onboarding or tutorial sequence, for a more immersive experience.
With Light theme, it looks too green.
Maybe you can keep this file (what has been done with Base theme is good and was missing in Riot, thanks), but either put the black color for android:navigationBarColor for all the themes, or remove the XML items. Note that for the Dark and Black theme, the primary_dark_color is not black but #191919.
|
|
||
| <!-- searches activity --> | ||
| <style name="SearchesAppTheme" parent="AppTheme"> | ||
| <style name="SearchesAppTheme" parent="AppTheme.Light"> |
There was a problem hiding this comment.
Maybe rename to SearchesAppTheme.Light?
| <item name="editTextColor">@android:color/white</item> | ||
| <item name="android:editTextColor">@android:color/white</item> | ||
| <item name="android:textColorHint">?attr/activity_bottom_gradient_color</item> | ||
| </style> | ||
|
|
||
| <style name="DirectoryPickerTheme" parent="AppTheme.NoActionBar" /> | ||
| <style name="DirectoryPickerTheme" parent="AppTheme.NoActionBar.Light" /> |
There was a problem hiding this comment.
Maybe rename to DirectoryPickerTheme.Light?
| </style> | ||
|
|
||
| <!-- call activity --> | ||
| <style name="CallActivityTheme" parent="AppTheme.Light"> |
There was a problem hiding this comment.
Maybe rename to CallActivityTheme.Light?
| </style> | ||
|
|
||
| <!-- home activity --> | ||
| <style name="HomeActivityTheme" parent="AppTheme.NoActionBar.Light"> |
There was a problem hiding this comment.
Maybe rename to HomeActivityTheme.Light?
| <item name="android:editTextStyle">@style/VectorSearches.EditText</item> | ||
|
|
||
| <!-- actionbar icons color --> | ||
| <item name="actionBarTheme">@style/VectorSearches.ActionBarTheme</item> | ||
| </style> | ||
|
|
||
| <style name="CallAppTheme" parent="AppTheme.NoActionBar"> | ||
| <style name="CallAppTheme" parent="AppTheme.NoActionBar.Light"> |
There was a problem hiding this comment.
Maybe rename to CallAppTheme.Light?
| <item name="android:background">@drawable/vector_tabbar_background_group_light</item> | ||
| <item name="background">@drawable/vector_tabbar_background_group_light</item> | ||
| </style> | ||
|
|
||
| <style name="GroupAppTheme" parent="AppTheme"> | ||
| <style name="GroupAppTheme" parent="AppTheme.Light"> |
There was a problem hiding this comment.
Maybe rename to GroupAppTheme.Light?
| android:layout_centerHorizontal="true" | ||
| android:layout_centerVertical="true" | ||
| android:src="@drawable/ic_material_play_circle" | ||
| android:background="#77777777" /> |
There was a problem hiding this comment.
Ok, to remove the background, it looks much better now.
Maybe you can take the opportunity of this PR to improve ic_material_play_circle.png which is so ugly :). I would also prefer to have wrap_content as layout width and height.
May I ask you if you could take this point please? (See #2577)
|
That should be everything. I removed the green navigation bar from the light theme, so now it is a light gray in the light theme and a dark gray in the dark theme. I also made a high resolution version of the play icon. I didn't want to redesign it since the android version is getting a redesign at some point. Might as well see what direction the iconography is going in first. I remember at some point you guys were going to try adding custom themes to the mobile apps like the web app, is that still planned? |
|
Noticed a lot of dialogs in the app had odd padding, found the issue which affects any AlertDialog.Builder using the setView method. I will fix them as I come across them. |
| videoView.setOnPreparedListener(new MediaPlayer.OnPreparedListener() { | ||
| @Override | ||
| public void onPrepared(MediaPlayer mp) { | ||
| mp.setLooping(true); |
There was a problem hiding this comment.
I understand this point of view, but maybe open a separate PR to introduce this change. In this PR it looks a bit offtopic :-)
Update matrix-sdk.aar lib - build 1884 - Revision: 4318cddf9a0caf6e6a401f73d258991add5ffdb1 Improvements: - Minor changes to toolbar style and other UI elements (#2529) - Improve intent to open document (#2544) - Avoid useless dialog for permission (#2331) - Improve wording when exporting keys (#2289) Other changes: - Upgrade lib libphonenumber from v8.0.1 to 8.9.12 - Upgrade Google firebase libs Bugfix: - Handle `\/` at the beginning of a message to send a message starting with `/` (#658) - Escape nicknames starting with a forward slash `/` in mentions (#2146) - Improve management of Push feature - MatrixError mResourceLimitExceededError is now managed in MxDataHandler (element-hq/riot-android#2547 point 2) Merge commit 'a6825201439cd5f248f122c6db909dd632366a33' into develop # Conflicts: # build.gradle # vector/libs/matrix-sdk.aar # vector/src/app/AndroidManifest.xml # vector/src/appfdroid/AndroidManifest.xml # vector/src/main/AndroidManifest.xml # vector/src/main/java/im/vector/Matrix.java # vector/src/main/java/im/vector/activity/CommonActivityUtils.java # vector/src/main/java/im/vector/activity/VectorHomeActivity.java # vector/src/main/java/im/vector/activity/VectorMemberDetailsActivity.java # vector/src/main/java/im/vector/activity/VectorRoomActivity.java # vector/src/main/java/im/vector/activity/VectorRoomInviteMembersActivity.java # vector/src/main/java/im/vector/fragments/VectorRoomSettingsFragment.java # vector/src/main/java/im/vector/fragments/VectorSettingsPreferencesFragment.kt # vector/src/main/java/im/vector/util/ExternalApplicationsUtil.kt # vector/src/main/res/layout/activity_home.xml # vector/src/main/res/layout/activity_vector_room.xml # vector/src/main/res/layout/adapter_local_contacts_sticky_header_subview.xml # vector/src/main/res/values-fr/strings.xml # vector/src/main/res/values/colors.xml # vector/src/main/res/values/styles.xml
I mainly made these changes to suggest that the icons in the dark theme be changed to white. This is the normal color for dark themes in other android apps, and the contrast in the dark theme is very bad at times in the HomeActivity toolbar. I also changed the navigation bar to a dark color so it wouldn't stand out as much on the dark themes. Here are some pictures below of before and after the color changes.
I also changed the hang up button in the call view to grey to match all the other buttons, it looked a bit odd in a different color to me.