Feature/experiments/go pro/multi select file upload #1205

sultanmyrza · 2022-01-28T07:22:26Z

┆Issue is synchronized with this Asana task by Unito
┆Created By: Tammy Yang

Bump version to 0.47.0

change button color to match design

remove unused imports, init logic.

package.json

scss-linting

shc261392 · 2022-02-07T07:10:13Z

src/app/features/home/home.page.ts

-            )
-        )
+        concatMap(async ([takePicture, recordVideo]) => {
+          // eslint-disable-next-line no-async-promise-executor


We follow eslint as much as possible whenever it is reasonable. Using async function as Promise executor is a common anti-pattern which sometimes causes some unexpected behavior so it is best avoided.
To use the getConnectedDevice() async function's result, you can use a combineLatest to combine it with the translteObject observable, see https://github.com/numbersprotocol/capture-lite/blob/master/src/app/features/home/details/details.page.ts#L254 for example

The goal here is to refactor this part so that // eslint-disable-next-line no-async-promise-executor won't be needed to pass lint

f071852 should resolve

shc261392 · 2022-02-07T07:12:19Z

.../go-pro/go-pro-media-item-detail-on-camera/go-pro-media-item-detail-on-camera.component.html

+    <ion-button
+      (click)="uploadToCapture()"
+      expand="block"
+      style="color: white; --box-shadow: 0; margin: 0 16px 0 16px"


It would be good if these styles can be put in the .scss file and keep inline styling usage minimal

hope this commit "move all html styles to" will resolve

shc261392 · 2022-02-07T09:37:00Z

...eatures/settings/go-pro/go-pro-media-list-on-camera/go-pro-media-list-on-camera.component.ts

+        // // eslint-disable-next-line no-console
+        // console.log(fileToUpload);
+
+        // await new Promise(resolve => {
+        //   setTimeout(() => {
+        //     resolve(fileToUpload);
+        //     // eslint-disable-next-line @typescript-eslint/no-magic-numbers
+        //   }, 4000);
+        // });


These code should be removed

03dd1d6 should resolve

shc261392 · 2022-02-07T09:39:38Z

...eatures/settings/go-pro/go-pro-media-list-on-camera/go-pro-media-list-on-camera.component.ts

+    const { role } = await alert.onDidDismiss();
+    // eslint-disable-next-line no-console
+    console.log('onDidDismiss resolved with role', role);


If role does not do anything here, it should be removed to avoid confusion.

a8e8dac should resolve

shc261392 · 2022-02-07T09:41:30Z

src/app/features/settings/go-pro/go-pro.page.html

+      (click)="scanForBluetoothDevices()"
+      style="color: white; --box-shadow: 0; margin: 0 16px 0 16px"
+    >
+      {{ bluetoothIsScanning ? 'Scaning' : 'Scan for bluetooth devices' }}


Suggested change

{{ bluetoothIsScanning ? 'Scaning' : 'Scan for bluetooth devices' }}

{{ bluetoothIsScanning ? 'Scanning' : 'Scan for bluetooth devices' }}

fix(go-pro.page): typo

shc261392 · 2022-02-07T09:46:16Z

src/app/features/settings/go-pro/go-pro.page.ts

+    try {
+      this.bluetoothIsScanning = true;
+      this.bluetoothScanResults =
+        await this.goProBluetoothService.scanForBluetoothDevices();
+      this.bluetoothIsScanning = false;
+    } catch (error) {
+      this.bluetoothScanResults = [];
+      this.bluetoothIsScanning = false;
+    }


Suggested change

try {

this.bluetoothIsScanning = true;

this.bluetoothScanResults =

await this.goProBluetoothService.scanForBluetoothDevices();

this.bluetoothIsScanning = false;

} catch (error) {

this.bluetoothScanResults = [];

this.bluetoothIsScanning = false;

}

try {

this.bluetoothIsScanning = true;

this.bluetoothScanResults =

await this.goProBluetoothService.scanForBluetoothDevices();

} catch (error) {

this.bluetoothScanResults = [];

} finally {

this.bluetoothIsScanning = false;

}

73668e0 should resolve

shc261392 · 2022-02-07T09:49:50Z

src/app/features/settings/go-pro/services/go-pro-bluetooth.service.ts

+  private readonly enableGoProWiFiCommand = [0x03, 0x17, 0x01, 0x01];
+
+  constructor() {
+    BleClient.initialize();


Because these services are provided in shared module which is loaded when the app/test starts running, I would suggest leave the service constructor empty, and do a late initialization when the Bluetooth-related functionalities are really needed

commit ccb1320 should resolve

shc261392 · 2022-02-07T09:59:15Z

src/app/features/settings/go-pro/services/go-pro-bluetooth.service.ts

+    await this.checkBluetoothDeviceConnection();
+
+    // TODO: find better solution for comparing 2 arrays with numbers
+    if (JSON.stringify(command) === JSON.stringify(this.shutdownCommand)) {


Suggested change

if (JSON.stringify(command) === JSON.stringify(this.shutdownCommand)) {

import { isEqual } from 'lodash-es';

...

if (isEqual(command, this.shutdownCommand)) {

We already uses lodash in other places so the best way to compare array or objects is by using lodash.isEqual

refactor(go-pro-bluetooth.service.ts): use isEqual

shc261392 · 2022-02-07T10:03:32Z

src/app/features/settings/go-pro/services/go-pro-media.service.ts

+    private readonly httpClient: HttpClient
+  ) {}
+
+  // eslint-disable-next-line class-methods-use-this


For all the lint rule class-methods-use-this issues, I would suggest define these functions in a standalone file and export them, instead of defining them as GoProMediaService's class methods. See src/app/utils/url.ts for example (it is also a good place to put url-handling functions)

2f0ce31 should resolve

shc261392 · 2022-02-07T10:32:16Z

src/app/features/settings/go-pro/services/go-pro-wifi.service.ts

+import { GoProBluetoothService } from './go-pro-bluetooth.service';
+const Wifi: WifiPlugin = Plugins.Wifi as WifiPlugin;
+
+const { Storage } = Plugins;


The App already has a PreferenceManager class which wraps the Storage plugin and makes values as subscriptables. Please use PreferenceManager directly instead of access Storage plugin directly here so that the App has a single uniform way to handle key-value storages.
You can check WebCryptoApiSignatureProvider for usage example for the PreferenceManager.

1688367 using PreferenceManager for WiFi

4e5d9a3 using PreferenceManager for BLE

However running locally npm run test.ci giving this error:

after commit 1688367

test logs

GoProWifiService should be created FAILED NullInjectorError: R3InjectorError(DynamicTestModule)[GoProWifiService -> PreferenceManager -> InjectionToken STORAGE_PLUGIN -> InjectionToken STORAGE_PLUGIN]: NullInjectorError: No provider for InjectionToken STORAGE_PLUGIN! error properties: Object({ ngTempTokenPath: null, ngTokenPath: [ 'GoProWifiService', 'PreferenceManager', 'InjectionToken STORAGE_PLUGIN', 'InjectionToken STORAGE_PLUGIN' ] }) NullInjectorError: R3InjectorError(DynamicTestModule)[GoProWifiService -> PreferenceManager -> InjectionToken STORAGE_PLUGIN -> InjectionToken STORAGE_PLUGIN]: NullInjectorError: No provider for InjectionToken STORAGE_PLUGIN! at NullInjector.get (http://localhost:9876/_karma_webpack_/vendor.js:74891:27) at R3Injector.get (http://localhost:9876/_karma_webpack_/vendor.js:75058:33) at R3Injector.get (http://localhost:9876/_karma_webpack_/vendor.js:75058:33) at injectInjectorOnly (http://localhost:9876/_karma_webpack_/vendor.js:68541:33) at ɵɵinject (http://localhost:9876/_karma_webpack_/vendor.js:68545:61) at Object.PreferenceManager_Factory [as factory] (ng:///PreferenceManager/ɵfac.js:4:45) at R3Injector.hydrate (http://localhost:9876/_karma_webpack_/vendor.js:75228:35) at R3Injector.get (http://localhost:9876/_karma_webpack_/vendor.js:75047:33) at injectInjectorOnly (http://localhost:9876/_karma_webpack_/vendor.js:68541:33) at ɵɵinject (http://localhost:9876/_karma_webpack_/vendor.js:68545:61) Error: Expected undefined to be truthy. at <Jasmine> at UserContext.<anonymous> (http://localhost:9876/_karma_webpack_/main.js:1131:25) at ZoneDelegate.invoke (http://localhost:9876/_karma_webpack_/polyfills.js:8285:26) at ProxyZoneSpec.onInvoke (http://localhost:9876/_karma_webpack_/vendor.js:340077:39)

after commit 4e5d9a3

test logs

[31mChrome Headless 98.0.4758.80 (Mac OS 10.15.7) GoProBluetoothService should be created FAILED NullInjectorError: R3InjectorError(DynamicTestModule)[GoProBluetoothService -> PreferenceManager -> InjectionToken STORAGE_PLUGIN -> InjectionToken STORAGE_PLUGIN]: NullInjectorError: No provider for InjectionToken STORAGE_PLUGIN! error properties: Object({ ngTempTokenPath: null, ngTokenPath: [ 'GoProBluetoothService', 'PreferenceManager', 'InjectionToken STORAGE_PLUGIN', 'InjectionToken STORAGE_PLUGIN' ] }) NullInjectorError: R3InjectorError(DynamicTestModule)[GoProBluetoothService -> PreferenceManager -> InjectionToken STORAGE_PLUGIN -> InjectionToken STORAGE_PLUGIN]: NullInjectorError: No provider for InjectionToken STORAGE_PLUGIN! at NullInjector.get (http://localhost:9876/_karma_webpack_/vendor.js:74891:27) at R3Injector.get (http://localhost:9876/_karma_webpack_/vendor.js:75058:33) at R3Injector.get (http://localhost:9876/_karma_webpack_/vendor.js:75058:33) at injectInjectorOnly (http://localhost:9876/_karma_webpack_/vendor.js:68541:33) at ɵɵinject (http://localhost:9876/_karma_webpack_/vendor.js:68545:61) at Object.PreferenceManager_Factory [as factory] (ng:///PreferenceManager/ɵfac.js:4:45) at R3Injector.hydrate (http://localhost:9876/_karma_webpack_/vendor.js:75228:35) at R3Injector.get (http://localhost:9876/_karma_webpack_/vendor.js:75047:33) at injectInjectorOnly (http://localhost:9876/_karma_webpack_/vendor.js:68541:33) at ɵɵinject (http://localhost:9876/_karma_webpack_/vendor.js:68545:61) Error: Expected undefined to be truthy. at <Jasmine> at UserContext.<anonymous> (http://localhost:9876/_karma_webpack_/main.js:1077:25) at ZoneDelegate.invoke (http://localhost:9876/_karma_webpack_/polyfills.js:8285:26) at ProxyZoneSpec.onInvoke (http://localhost:9876/_karma_webpack_/vendor.js:340077:39)

method from lodash to compare objects

bluetooth plugin

to wrap scanForBluetooth call

remove unused comments

remove unused variable

methods to src/utils/url.ts

instead of Plugins.Storage

use preferenceManager instead of Plugins.Storage

… device

shc261392

Looks good to me

shc261392 and others added 9 commits January 27, 2022 16:18

Merge pull request #1198 from numbersprotocol/develop

5aa7b3f

Bump version to 0.47.0

remove unused GoPro UI components

fe1813a

style(go-pro-media-item-detail-on-camera.component.html)

d994209

change button color to match design

fix(go-pro-media-item-detail-on-camera.component.ts)

06808dd

remove unused imports, init logic.

refactor(go-pro-bluetooth.service.ts)

6b9a1d6

style(go-pro.page.html)

7b8786e

feat: trigger bluetooth pairing at first connection

4a53b02

change(go-pro-media.servie.ts): refactor, remove unused methods

ffaee21

feat(go-pro-media-list-on-camera): multi-select upload

bee915c

sultanmyrza commented Jan 28, 2022

View reviewed changes

package.json Show resolved Hide resolved

sultanmyrza added 2 commits January 28, 2022 16:29

add missing "node_modules/ngx-long-press2"

e045837

fix(go-pro-media-list-on-camera.component.scss):

df37236

scss-linting

shc261392 requested changes Feb 7, 2022

View reviewed changes

sync-by-unito bot mentioned this pull request Feb 8, 2022

Code review and merge GoPro feature #1184

Closed

sultanmyrza added 15 commits February 8, 2022 16:42

move html styles to scss classes

c98c7a0

fix(go-pro.page): typo

66191db

refactor(go-pro-bluetooth.service.ts): use isEqual

d03c833

method from lodash to compare objects

refactor(go-pro-bluetooth.service.ts): lazy init

ccb1320

bluetooth plugin

refactor(go-pro-page.ts): use try-catch-finally block

73668e0

to wrap scanForBluetooth call

refactor(go-pro-media-list-on-camera.component.ts)

03dd1d6

remove unused comments

refactor(go-pro-media-list-on-camera.component.ts):

a8e8dac

remove unused variable

refactor(go-pro.page.ts): remove unused comments

5c98461

refactor(go-pro-media.service.ts): remove unused methods

8c26a53

refactor(go-pro-media.service.ts): move url utility

2f0ce31

methods to src/utils/url.ts

refactor(go-pro-wifi.service.ts): use preferenceManager

1688367

instead of Plugins.Storage

refactor(go-pro-bluetooth.service.ts):

4e5d9a3

use preferenceManager instead of Plugins.Storage

refactor(home.page.ts): fix no-async-promise-executor

f071852

fix(go-pro-bluetooth.service.ts): disconnect before connection to new…

cd713b0

… device

fix test related to GoPro Bluetoot and WiFi services

34ea1af

shc261392 approved these changes Mar 11, 2022

View reviewed changes

shc261392 merged commit 487e684 into numbersprotocol:feature/experiments/go-pro/beta/1.0 Mar 11, 2022

ruizehung mentioned this pull request Apr 7, 2022

Feature/experiments/go pro/beta/1.0 #1368

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/experiments/go pro/multi select file upload #1205

Feature/experiments/go pro/multi select file upload #1205

sultanmyrza commented Jan 28, 2022 •

edited by sync-by-unito bot

Loading

shc261392 Feb 7, 2022

sultanmyrza Feb 12, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 8, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 10, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 10, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 8, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 10, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 10, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 8, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 10, 2022

shc261392 Feb 7, 2022

sultanmyrza Feb 12, 2022

shc261392 left a comment

	{{ bluetoothIsScanning ? 'Scaning' : 'Scan for bluetooth devices' }}
	{{ bluetoothIsScanning ? 'Scanning' : 'Scan for bluetooth devices' }}

-    if (JSON.stringify(command) === JSON.stringify(this.shutdownCommand)) {
+    import { isEqual } from 'lodash-es';
+    ...
+    if (isEqual(command, this.shutdownCommand)) {

Feature/experiments/go pro/multi select file upload #1205

Feature/experiments/go pro/multi select file upload #1205

Conversation

sultanmyrza commented Jan 28, 2022 • edited by sync-by-unito bot 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

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

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

shc261392 left a comment

Choose a reason for hiding this comment

sultanmyrza commented Jan 28, 2022 •

edited by sync-by-unito bot

Loading