MediaPreviewer : clean files and manage bitmap with Glide #2445

ganfra · 2018-07-18T13:58:37Z

Some improvements over the MediaPreviewer classes

bmarty · 2018-07-18T14:54:34Z

Search for forbidden patterns in code...
❌ Error: '.{161}' detected 1 time, in file './vector/src/main/java/im/vector/activity/MediaPreviewerActivity.java'.
Rule: Line length is limited to 160 chars. Please split long lines
1 forbidden pattern detected

bmarty

A few remarks

bmarty · 2018-07-18T14:58:04Z

vector/src/main/java/im/vector/activity/MediaPreviewerActivity.java

-            statusBar.setBackgroundColor(color);
-        }
+    @TargetApi(Build.VERSION_CODES.LOLLIPOP)
+    private void setStatusBarColor(@ColorRes int statusBarColor) {


Remove then?

bmarty · 2018-07-18T14:58:34Z

vector/src/main/java/im/vector/adapters/MediaPreviewAdapter.java

     */
-    public MediaPreviewAdapter(List<RoomMediaMessage> imagePreviewList, ItemPositionChangedListener itemPositionChangedListener) {
+    public MediaPreviewAdapter(final List<RoomMediaMessage> imagePreviewList, EventListener mEventListener) {


param name convention

bmarty · 2018-07-18T14:58:44Z

vector/src/main/java/im/vector/adapters/MediaPreviewAdapter.java

-            super(itemView);
-            ButterKnife.bind(this, itemView);
-        }
+        this.mEventListener = mEventListener;


remove this.

bmarty · 2018-07-18T15:02:24Z

vector/src/main/res/layout/activity_media_previewer.xml


    <TextView
-        android:id="@+id/file_name"
+        android:id="@+id/media_previewer_file_name"


No need for a background here? (I've not run it)

the text is hardly visible, maybe add transparent background

Yep, added.

bmarty · 2018-07-18T15:02:42Z

vector/src/main/java/im/vector/adapters/MediaPreviewAdapter.java

+        void onMediaMessagePreviewClicked(RoomMediaMessage roomMediaMessage);
+    }
+


remove extra new lines pls

bmarty · 2018-07-18T15:11:41Z

no more crashes when selecting a large number of media?
-> Tested with 25 images with success

bmarty · 2018-07-18T15:35:48Z

vector/src/main/java/im/vector/activity/MediaPreviewerActivity.java

+            mPreviewerRecyclerView.setLayoutManager(linearLayoutManager);
+            final MediaPreviewAdapter mediaPreviewAdapter = new MediaPreviewAdapter(sharedDataItems, this);
+            mPreviewerRecyclerView.setAdapter(mediaPreviewAdapter);
+            updatePreview(sharedDataItems.get(0));


It's strange, it's working, but I think this line should be outside the else block, no?

Actually it's not working if you select "Use native camera" in the settings if if you take a photo. Can you check this case pls?

Yes, just saw that too

bmarty · 2018-07-18T17:44:32Z

vector/src/main/java/im/vector/activity/MediaPreviewerActivity.java

+        final MediaPreviewAdapter mediaPreviewAdapter = new MediaPreviewAdapter(sharedDataItems, this);
+        mPreviewerRecyclerView.setAdapter(mediaPreviewAdapter);
+
+        if (!sharedDataItems.isEmpty()) {


I think this cannot be empty, no?

MediaPreviewer : clean files and manage bitmap with Glide

3076a0b

ganfra requested a review from bmarty July 18, 2018 13:58

bmarty suggested changes Jul 18, 2018

View reviewed changes

bmarty reviewed Jul 18, 2018

View reviewed changes

ganfra added 4 commits July 18, 2018 18:13

MediaPreviewer : add more cleaning and fix bad checking

1e19d77

MediaPreviewer : add background color to file name view

c10b4b3

MediaPreviewer : update CHANGES.rst file

54a2549

MediaPreviewer : optimize imports

cdd651b

bmarty approved these changes Jul 18, 2018

View reviewed changes

MediaPreviewer : finish cleaning files

5ff6ea4

ganfra merged commit cc1d695 into develop Jul 19, 2018

ganfra deleted the feature/media_previewer_review branch July 19, 2018 09:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MediaPreviewer : clean files and manage bitmap with Glide #2445

MediaPreviewer : clean files and manage bitmap with Glide #2445

ganfra commented Jul 18, 2018

bmarty commented Jul 18, 2018

bmarty left a comment

bmarty Jul 18, 2018

ganfra Jul 18, 2018

bmarty Jul 18, 2018

bmarty Jul 18, 2018

bmarty Jul 18, 2018

bmarty Jul 18, 2018

ganfra Jul 18, 2018

bmarty Jul 18, 2018

bmarty commented Jul 18, 2018 •

edited

Loading

bmarty Jul 18, 2018

bmarty Jul 18, 2018 •

edited

Loading

ganfra Jul 18, 2018

bmarty Jul 18, 2018

ganfra Jul 19, 2018

		void onMediaMessagePreviewClicked(RoomMediaMessage roomMediaMessage);
		}

MediaPreviewer : clean files and manage bitmap with Glide #2445

MediaPreviewer : clean files and manage bitmap with Glide #2445

Conversation

ganfra commented Jul 18, 2018

bmarty commented Jul 18, 2018

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty commented Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

bmarty Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty commented Jul 18, 2018 •

edited

Loading

bmarty Jul 18, 2018 •

edited

Loading