Feat/flow 8078 savencia #29

lmancel · 2020-11-06T12:47:02Z

No description provided.

proper toggle and key forwarding to wexond and flowr TODO: handle input field focus/blur

TODO: handle the switch between wexond (tab input) and Views We need to be able to have a single timeout for all components

Automatic build reaches timeout after 10 minutes when generating the dmg

pls add usage examples for this command...

ArnaudGathy · 2020-11-06T13:04:49Z

src/application-manager/application-manager.ts

@@ -119,16 +94,16 @@ export class ApplicationManager {

  initLocalApps(clearStore: boolean = false): Promise<void[]> {
    return new Promise((resolve, reject) => {
-      fs.readdir(join(app.getAppPath(), 'build', 'applications'), (err, files) => {
+      // TODO: open ticket @ electron "withFileTypes" option is not working inside of the asar archive, it only returns the file names


What about we create a ticket with that task 😊

Actually already done by other peoples: electron/electron#25349

Also cf electron/electron#24062

ArnaudGathy · 2020-11-06T13:10:26Z

src/applications/keyboard/keyboardWindow.ts

+      keyPress: this.onKeyPress.bind(this),
+      keyUp: this.onKeyUp.bind(this),
+    }
+    Object.entries(ipcEvents).forEach(event => ipcMain.on(...event))


Neat syntax 👌

ArnaudGathy · 2020-11-06T13:12:13Z

src/applications/keyboard/preload.ts

+  global.ipcRenderer = ipcRenderer
+})
+
+export {}


Why ? What does it do ?

Hmmmmm right... I think it was to be able to use a declare statement above so it's considered as a module, but since there already is an import statement I might be able to remove it

ArnaudGathy · 2020-11-06T13:18:12Z

src/applications/keyboard/views/style.css

@@ -0,0 +1,3 @@
+.hg-button.hg-highlight {
+    box-shadow: 0 0 40px 5px rgba(64, 150, 255, 0.696);


That's an oddly specific value for opacity 🤔

Just used the color selector from VSCode, it's more or less random 😄

ArnaudGathy · 2020-11-06T13:20:53Z

src/barcoKeyboard/barcoKeyBoardController.ts

+  keyboardWindow?: KeyboardWindow
+
+  get isEnabled(): boolean {
+    return this.flowrStore?.get('enableVirtualKeyboard') ?? false


Suggested change

return this.flowrStore?.get('enableVirtualKeyboard') ?? false

return !!this.flowrStore?.get('enableVirtualKeyboard')

ArnaudGathy · 2020-11-06T13:29:42Z

src/barcoKeyboard/keyboardOpener.ts

+import { VirtualKeyboardEvent } from './events'
+
+// tslint:disable-next-line: function-name
+export function KeyboardOpener<T extends new (...args: any[]) => {}>(yo: T) {


Is it missing a = to make an arrow function => or is it some syntax i've never seen ?
{}>(yo: T)

I'll delete this file, it was an attempt that was unneeded in the end

ArnaudGathy · 2020-11-06T13:31:03Z

src/barcoKeyboard/keyboardOpener.ts

+
+// tslint:disable-next-line: function-name
+export function KeyboardOpener<T extends new (...args: any[]) => {}>(yo: T) {
+  return class extends yo {


What is yo ?

ArnaudGathy · 2020-11-06T13:39:40Z

src/wexond/main/view.ts

+import { parse } from 'tldts'
+import { buildPreloadPath } from '../../common/preload'
+
+// function BrowserWindowExtend(electronClass: BrowserWindow) {


Is this big chunk of commented code needed ?

No it's not, I did not push the message I wrote earlier but it's going to be removed.

It's related to the fact that extending electron's base classes is not supported, so I wanted to create a simple class that would implement the exact same interface and just act as a bridge. Might be needed in the future but overkill for now.

ArnaudGathy · 2020-11-06T13:49:13Z

src/wexond/renderer/app/components/App/index.tsx


 const GlobalStyle = createGlobalStyle`${Style}`;

 window.onbeforeunload = () => {
  ipcRenderer.send('browserview-clear');
 };

-export const App = observer(() => {
+export default observer((data: App) => {


Why this change ? Just to not have the same name as App in models/app ?

Actually I just used the same logic than what was done in other Wexond components without even trying to think of something better...

ArnaudGathy · 2020-11-06T13:55:16Z

src/wexond/renderer/app/components/Toolbar/index.tsx

+          <NavigationButtons />
+          {buttons}
+        </StyledToolbar>
+    )
  }

  return (
      <StyledToolbar isHTMLFullscreen={store.isHTMLFullscreen}>
        <NavigationButtons />
        <Tabbar />
        <Find />


The only difference between the two renders are <Tabbar /> and <Find />
It would have been much easier and less awkward to condtionnaly render them instead of having two returns and all the buttons stored in a variable

{!data.disableTabs && (<> <Tabbar /> <Find /> </>)}

lmancel · 2020-11-06T12:50:43Z

webpack.main.js

+const { getOptimization, webpackModule, resolve, Mode, output, deleteDir, OUTPUT_DIR } = require('./webpack/utils')
+
+module.exports = async (env) => {
+    await deleteDir(OUTPUT_DIR)


Will replace with CleanWebpackPlugin

lmancel · 2020-11-06T12:52:59Z

webpack.config.js

@@ -0,0 +1,249 @@
+


Will delete this file

lmancel · 2020-11-06T12:53:41Z

static/pages/config.html

+			new URL(url)
+			ipc.send('setExtUrl', url)
+		} catch (e) {
+			invalidUrl()


Will add console.error for debugging

lmancel · 2020-11-06T12:54:21Z

src/wexond/main/view.ts

+import { parse } from 'tldts'
+import { buildPreloadPath } from '../../common/preload'
+
+// function BrowserWindowExtend(electronClass: BrowserWindow) {


This was wip, I will remove it

Azevedo-Dani · 2020-11-09T07:38:51Z

src/applications/keyboard/keyboardWindow.ts

+}
+
+export class KeyboardWindow extends BrowserWindow {
+  capsLock: boolean = false


Suggested change

capsLock: boolean = false

capsLock = false

Azevedo-Dani · 2020-11-09T07:45:13Z

src/applications/keyboard/views/index.ts

+          class: 'hg-highlight',
+          buttons: SimpleKeyboardKeys.CAPS_LOCK,
+        }]
+      case KeyboardState.DEFAULT:


Suggested change

case KeyboardState.DEFAULT:

Azevedo-Dani · 2020-11-09T07:52:31Z

src/applications/keyboard/views/index.ts

+            return KeyboardState.CAPS_LOCK
+        }
+      default:
+        switch (this.state) {


We don't have a default case for this switch ?

The default case is supposed to do nothing. It could be replaced with an if statement, I just kept the same syntax as for the rest of the function

Azevedo-Dani · 2020-11-09T08:07:30Z

src/inactivity/window.ts

+import { ACTIVITY_EVENT, INACTIVITY_THROTTLE } from './utils'
+
+export async function watchForInactivity(baseWindow: BrowserWindow | BrowserView, timeoutDuration: number, callback: (baseWindow: BrowserWindow | BrowserView) => any) {
+  let timeout: number | undefined = undefined


Suggested change

let timeout: number | undefined = undefined

let timeout?: number

I can't use this syntax, it's not a class property

lmancel added 9 commits October 22, 2020 15:58

feat(keyboard) first keyboard impl

e1568d2

proper toggle and key forwarding to wexond and flowr TODO: handle input field focus/blur

tmp

21b8ab8

fix(inactivity) handle views inactivity in manager

fa3760d

TODO: handle the switch between wexond (tab input) and Views We need to be able to have a single timeout for all components

feat: inactivity + keyboard + no tabs

b6eec89

updated README

e4ff08a

Attempt at fixing automatic build

f847595

split regular and widevine build config

df40d18

wait 20minutes instead of 10 for osx build

a1edc96

Automatic build reaches timeout after 10 minutes when generating the dmg

Maybe fixed syntax of travis_wait ?

a0976bc

pls add usage examples for this command...

lmancel requested review from ArnaudGathy and Azevedo-Dani November 6, 2020 12:48

ArnaudGathy reviewed Nov 6, 2020

View reviewed changes

lmancel commented Nov 6, 2020

View reviewed changes

Azevedo-Dani reviewed Nov 9, 2020

View reviewed changes

fix(various) syntax, typos, comments

fafc618

Azevedo-Dani approved these changes Nov 16, 2020

View reviewed changes

ArnaudGathy approved these changes Nov 16, 2020

View reviewed changes

lmancel merged commit 56f41b4 into base Nov 17, 2020

lmancel deleted the feat/FLOW-8078-savencia branch November 17, 2020 07:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/flow 8078 savencia #29

Feat/flow 8078 savencia #29

lmancel commented Nov 6, 2020

ArnaudGathy Nov 6, 2020 •

edited

lmancel Nov 16, 2020

ArnaudGathy Nov 6, 2020

ArnaudGathy Nov 6, 2020

lmancel Nov 6, 2020

ArnaudGathy Nov 6, 2020

lmancel Nov 6, 2020

ArnaudGathy Nov 6, 2020

ArnaudGathy Nov 6, 2020

lmancel Nov 6, 2020

ArnaudGathy Nov 6, 2020

ArnaudGathy Nov 6, 2020

lmancel Nov 6, 2020

ArnaudGathy Nov 6, 2020

lmancel Nov 6, 2020 •

edited

ArnaudGathy Nov 6, 2020

lmancel Nov 6, 2020

lmancel Nov 6, 2020

lmancel Nov 6, 2020

lmancel Nov 6, 2020

Azevedo-Dani Nov 9, 2020

Azevedo-Dani Nov 9, 2020

Azevedo-Dani Nov 9, 2020

lmancel Nov 16, 2020 •

edited

Azevedo-Dani Nov 9, 2020

lmancel Nov 16, 2020 •

edited

		@@ -0,0 +1,3 @@
		.hg-button.hg-highlight {
		box-shadow: 0 0 40px 5px rgba(64, 150, 255, 0.696);

	return this.flowrStore?.get('enableVirtualKeyboard') ?? false
	return !!this.flowrStore?.get('enableVirtualKeyboard')

	let timeout: number \| undefined = undefined
	let timeout?: number

Feat/flow 8078 savencia #29

Feat/flow 8078 savencia #29

Conversation

lmancel commented Nov 6, 2020

ArnaudGathy Nov 6, 2020 • edited

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

lmancel Nov 6, 2020 • edited

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

lmancel Nov 16, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmancel Nov 16, 2020 • edited

Choose a reason for hiding this comment

ArnaudGathy Nov 6, 2020 •

edited

lmancel Nov 6, 2020 •

edited

lmancel Nov 16, 2020 •

edited

lmancel Nov 16, 2020 •

edited