Fixes #638 - updated camera #1747

ianscott313 · 2018-01-25T05:48:42Z

I accidentally switched off of the develop branch on to the master when i started this so its based off master. The only existing files i really changed had to do with the MediaPickerViewController which were completely taken out so I believe it should merge alright

Also 'Find Implicit Dependencies' needs to be unchecked in Edit Scheme > Build

**UPDATE: I pulled down develop, locally merged my feature branch with that, resolved conflicts, and pushed it back up. We should be good to go now

Signed-off-by: Ian Scott [email protected]

Fixes #638

ianscott313 · 2018-01-26T14:12:59Z

**UPDATE: I pulled down develop, locally merged my feature branch with that, resolved conflicts, and pushed it back up. We should be good to go now

giomfo

@ianscott313: I did a first review of your PR (sorry for the delay ;)), it is a global review because I was not able to build Riot project from your sources. I got an error in RiotShareExtension:
"MatrixKit/MatrixKit.h file not found".
Could you please fix this compilation issue?
(You should read first all my review before working on this compilation issue, I mentioned some points that may help you).

Here are some others global remarks:

You introduced a large part of code from third parties. You should do that only when it is really necessary. Some functionalities were already available, we would prefer to improve the existing ones rather than adding new files.
you should update podfile.lock, the checksum is wrong since I resolved a conflict in it.
icons: Could you please re-use the existing ones, and add only those which are really missing

giomfo · 2018-02-20T14:08:46Z

Podfile

+    pod 'OLMKit'
+    #pod 'OLMKit', :path => '../olm/OLMKit.podspec'
+    pod 'Realm', '~> 3.0.1'
+    pod 'LLSimpleCamera', :inhibit_warnings => true


Remove OLMKit and Realm pods here, they are handled at SDK level now

giomfo · 2018-02-20T14:17:54Z

Riot.xcodeproj/project.pbxproj

+		3C9CA406200C651F006031F1 /* [email protected] in Resources */ = {isa = PBXBuildFile; fileRef = 3C9CA403200C651E006031F1 /* [email protected] */; };
+		3C9CA407200C651F006031F1 /* [email protected] in Resources */ = {isa = PBXBuildFile; fileRef = 3C9CA404200C651E006031F1 /* [email protected] */; };
+		3C9CA408200C651F006031F1 /* send.png in Resources */ = {isa = PBXBuildFile; fileRef = 3C9CA405200C651E006031F1 /* send.png */; };
+		40D7564A9783FD6C28C157CE /* libPods-RiotPods-RiotShareExtension.a in Frameworks */ = {isa = PBXBuildFile; fileRef = AF0A0746EF74FF15B8B79658 /* libPods-RiotPods-RiotShareExtension.a */; };


I don't understand why and how you added libPods-RiotPods-RiotShareExtension.a here.
I think this is to hide a compilation error observed in RiotShareExtension since you unchecked 'Find Implicit Dependencies' in Edit Scheme > Build.

For your information, this added lib was automatically removed when I built locally the project.

giomfo · 2018-02-20T14:19:51Z

Riot.xcodeproj/project.pbxproj

@@ -382,8 +399,6 @@
 		F083BE141E7009ED00A9B29C /* HomeViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = F083BC311E7009EC00A9B29C /* HomeViewController.m */; };
 		F083BE151E7009ED00A9B29C /* MediaAlbumContentViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = F083BC331E7009EC00A9B29C /* MediaAlbumContentViewController.m */; };
 		F083BE161E7009ED00A9B29C /* MediaAlbumContentViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = F083BC341E7009EC00A9B29C /* MediaAlbumContentViewController.xib */; };
-		F083BE171E7009ED00A9B29C /* MediaPickerViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = F083BC361E7009EC00A9B29C /* MediaPickerViewController.m */; };
-		F083BE181E7009ED00A9B29C /* MediaPickerViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = F083BC371E7009EC00A9B29C /* MediaPickerViewController.xib */; };
 		F083BE191E7009ED00A9B29C /* RecentsViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = F083BC391E7009EC00A9B29C /* RecentsViewController.m */; };


Please do not remove the existing sources related to the MediaPickerViewController.

giomfo · 2018-02-20T14:22:56Z

Riot.xcodeproj/project.pbxproj

@@ -2770,7 +2833,7 @@
 				TargetAttributes = {
 					24CBEC4D1F0EAD310093EABB = {
 						CreatedOnToolsVersion = 8.3.2;
-						DevelopmentTeam = 7J4U792NQT;
+                        DevelopmentTeam = 7J4U792NQT;


Could you please remove these useless diff due to tabulation changes?

giomfo · 2018-02-20T14:26:30Z

Riot.xcodeproj/project.pbxproj

@@ -4115,10 +4192,11 @@
 			isa = XCBuildConfiguration;
 			baseConfigurationReference = A5030B7C3C0B6EB83A9257BD /* Pods-RiotPods-Riot.debug.xcconfig */;
 			buildSettings = {
+				APPLICATION_EXTENSION_API_ONLY = NO;


Can you explain why and how this build setting APPLICATION_EXTENSION_API_ONLYis appeared for the target named Riot?

giomfo · 2018-02-20T15:49:17Z

Riot/ViewController/CameraViewController.h

+//  Created by Ian on 1/12/18.
+//  Copyright © 2018 matrix.org. All rights reserved.
+//
+


Please use the following header template for each files you created

/*
Copyright 2018 <Your Name>

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

giomfo · 2018-02-20T15:51:15Z

Riot/ViewController/CameraViewController.m

+//
+//  Created by Ian on 1/12/18.
+//  Copyright © 2018 matrix.org. All rights reserved.
+//


See my previous comment about file header

giomfo · 2018-02-20T16:06:48Z

Riot/Utils/Camera/UIImage+Crop.h

+
+#import <UIKit/UIKit.h>
+
+@interface UIImage(CropCategory)


Please remove this category UIImage+Crop from the project sources, it is not use...

giomfo · 2018-02-20T16:19:12Z

Riot/ViewController/MediaPickerViewController.h

@@ -1,111 +0,0 @@
-/*


Please do not remove the files related to MediaPickerViewController

giomfo · 2018-02-20T16:54:19Z

Riot/ViewController/CameraViewController.m

+{
+    [super viewWillAppear:animated];
+
+    // Screen tracking (via Google Analytics)


Google Analytics use has been removed. We use now Piwik tracker.
You may replace the following lines by the simple one:
[[AppDelegate theDelegate] trackScreen:@"Camera"];

manuroe · 2019-08-08T12:34:04Z

We have merged #2620 for this feature.

element-hq#638 - updated camera

491ac62

giomfo self-requested a review January 25, 2018 18:43

fixed merge conflicts

984df95

ianscott313 and others added 5 commits February 3, 2018 16:26

Merge branch 'develop' into feature/638-UpdateCamera

a8077a2

Merge branch 'develop' into feature/638-UpdateCamera

ecba0c7

Merge branch 'develop' into feature/638-UpdateCamera

793a921

Merge branch 'develop' into feature/638-UpdateCamera

314f469

Merge branch 'develop' into feature/638-UpdateCamera

4a0a5be

giomfo requested changes Feb 20, 2018

View reviewed changes

manuroe closed this Aug 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #638 - updated camera #1747

Fixes #638 - updated camera #1747

ianscott313 commented Jan 25, 2018 •

edited

Loading

ianscott313 commented Jan 26, 2018

giomfo left a comment

giomfo Feb 20, 2018

giomfo Feb 20, 2018

giomfo Feb 20, 2018

giomfo Feb 20, 2018

giomfo Feb 20, 2018

giomfo Feb 20, 2018

giomfo Feb 20, 2018

giomfo Feb 20, 2018

giomfo Feb 20, 2018

giomfo Feb 20, 2018

manuroe commented Aug 8, 2019


		#import <UIKit/UIKit.h>

		@interface UIImage(CropCategory)

Fixes #638 - updated camera #1747

Fixes #638 - updated camera #1747

Conversation

ianscott313 commented Jan 25, 2018 • edited Loading

ianscott313 commented Jan 26, 2018

giomfo 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuroe commented Aug 8, 2019

ianscott313 commented Jan 25, 2018 •

edited

Loading