feat: add splash screen on top of ctx refactor #2548

SgtPooki · 2023-07-08T01:38:02Z

Depends on #2378 on the feat/ctx-refactor branch

feat: add splash screen
tmp: trying to fix e2e tests

Currently failing e2e tests.

Demo

SgtPooki

self review

src/splash/splash.html

src/webui/index.js

SgtPooki · 2023-07-08T02:02:14Z

src/webui/index.js

@@ -183,17 +187,34 @@ module.exports = async function () {
  })

  const launchWebUI = ctx.getFn('launchWebUI')
+  const splashScreenPromise = ctx.getProp('splashScreen')
+  if (store.get(CONFIG_KEY) && process.env.NODE_ENV !== 'test') {


showing the splash screen during testing causing some odd issues with playwright electron and not being able to get the actual webui window.

i wonder why that is? 🤔

i wonder why that is? 🤔

I think there is some testing that defaults to the first window returned, and if that window is the splash window, the tests fail. I removed the block on TEST and have more tests passing now.

test/e2e/launch.e2e.test.js

whizzzkid

nits. not blocking per se, but can be improved.

whizzzkid · 2023-07-08T09:08:45Z

src/splash/create-splash-screen.js

+  await splashScreen.loadFile(path.join(__dirname, './splash.html'))
+  splashScreen.center()


move this in the try block too?

src/splash/splash.html

whizzzkid · 2023-07-08T09:14:06Z

src/webui/index.js

@@ -183,17 +187,34 @@ module.exports = async function () {
  })

  const launchWebUI = ctx.getFn('launchWebUI')
+  const splashScreenPromise = ctx.getProp('splashScreen')
+  if (store.get(CONFIG_KEY) && process.env.NODE_ENV !== 'test') {


i wonder why that is? 🤔

src/webui/index.js

src/splash/create-splash-screen.js

* make keyframe animation dir normal * remove visual artifacts of drop-shadow on size reduction * make drop-shadow animation better

SgtPooki

self review

src/splash/splash.html

SgtPooki · 2023-07-31T20:25:34Z

src/webui/index.js

@@ -183,17 +187,34 @@ module.exports = async function () {
  })

  const launchWebUI = ctx.getFn('launchWebUI')
+  const splashScreenPromise = ctx.getProp('splashScreen')
+  if (store.get(CONFIG_KEY) && process.env.NODE_ENV !== 'test') {


i wonder why that is? 🤔

I think there is some testing that defaults to the first window returned, and if that window is the splash window, the tests fail. I removed the block on TEST and have more tests passing now.

SgtPooki · 2023-08-01T00:32:38Z

src/daemon/config.js

+  if (process.env.NODE_ENV === 'test' || process.env.CI != null) {
+    logger.info('[daemon] CI or TEST mode, skipping busyPortDialog')
+    promptUser = false
+  }


tests fail locally or timeout because of waiting on this prompt.

SgtPooki · 2023-08-01T00:33:32Z

src/splash/create-splash-screen.js

+    width: 250,
+    height: 275,


reduced size due to background page being gone, but it needs to be bigger than the 200x200 px icon

SgtPooki · 2023-08-01T00:34:17Z

src/webui/index.js

@@ -107,6 +112,7 @@ const createWindow = () => {
  })

  app.on('before-quit', () => {
+    logger.info('[web ui] app-quit requested')


tests have some race condition where playwright will shut down electron but then use the same instance to try to load webui pages... this helped me debug

SgtPooki · 2023-08-01T00:34:42Z

src/webui/index.js

+    if (window.isDestroyed()) {
+      logger.error(`[web ui] window is destroyed, not launching web ui with ${path}`)
+      return
+    }


e2e tests trying to launch webui even though it already killed electron and is done with the tests.

SgtPooki · 2023-08-01T00:36:08Z

src/webui/index.js

+  window.on('close', () => {
+    if (splashScreenTimeoutId) {
+      clearTimeout(splashScreenTimeoutId)
+      splashScreenTimeoutId = null
+    }
+  })


when app is closed, clear the timeout.. we don't need to handle splashscreen

won't it be auto GCed? I'm trying to understand the benefit of cleaning this up prior to close.

it should be, but it still causes issues in e2e due to some race condition in how playwright runs electron in our e2e tests.

src/context.js

src/splash/create-splash-screen.js

src/splash/splash.html

whizzzkid

nits, approved.

assets/pages/splash.html

src/context.js

whizzzkid · 2023-08-01T08:54:27Z

src/types.d.ts

@@ -0,0 +1 @@
+export type CONFIG_KEYS = 'AUTO_LAUNCH' | 'AUTO_GARBAGE_COLLECTOR' | 'SCREENSHOT_SHORTCUT' | 'OPEN_WEBUI_LAUNCH' | 'MONOCHROME_TRAY_ICON' | 'EXPERIMENT_PUBSUB' | 'EXPERIMENT_PUBSUB_NAMESYS'


src/webui/index.js

whizzzkid · 2023-08-01T08:58:32Z

src/webui/index.js

+  window.on('close', () => {
+    if (splashScreenTimeoutId) {
+      clearTimeout(splashScreenTimeoutId)
+      splashScreenTimeoutId = null
+    }
+  })


won't it be auto GCed? I'm trying to understand the benefit of cleaning this up prior to close.

Co-authored-by: Nishant Arora <[email protected]>

src/context.js

SgtPooki · 2023-08-01T19:44:41Z

src/webui/index.js

+  window.on('close', () => {
+    if (splashScreenTimeoutId) {
+      clearTimeout(splashScreenTimeoutId)
+      splashScreenTimeoutId = null
+    }
+  })


it should be, but it still causes issues in e2e due to some race condition in how playwright runs electron in our e2e tests.

src/webui/index.js

SgtPooki · 2023-08-01T22:16:17Z

these e2e tests are so sketchy.. they don't run consistently locally. they don't run consistently in github actions.. we should probably remove any e2e tests that are dependent upon ipfsd-ctl, because that's where i'm seeing a lot of problems.

Also, the ipfsd-ctl configs are still using ipfs-http-client because electron struggles with ESM :|

SgtPooki added 11 commits January 16, 2023 23:44

chore: update ctx refactor

7c02e34

fix: tests

b947d61

feat: add context.spec.js tests

042f858

fix: startup issues

472219c

feat: app context supports lazy functions

93a2bac

chore: self-PR-review cleanup

94e752e

Merge branch 'main' into feat/ctx-refactor

b6124fc

Merge branch 'main' into feat/ctx-refactor

221612d

test: fix e2e tests

07ff889

feat: add splash screen

af05f6d

tmp: trying to fix e2e tests

b3fb596

SgtPooki changed the base branch from main to feat/ctx-refactor July 8, 2023 01:38

SgtPooki added 3 commits July 7, 2023 18:55

chore(deps): upgrade playwright

118825c

fix: revert daemonReady function

c160aa0

Merge branch 'feat/ctx-refactor' into feat/ctx-refactor-splash-screen

5f444f0

SgtPooki commented Jul 8, 2023

View reviewed changes

SgtPooki added 9 commits July 7, 2023 19:05

Update src/splash/splash.html

6f05030

Update src/splash/splash.html

a3982ea

Update src/splash/splash.html

675166e

Update src/webui/index.js

d7e4d83

Update test/e2e/launch.e2e.test.js

241a0aa

Update test/e2e/launch.e2e.test.js

5dfd14e

Update test/e2e/launch.e2e.test.js

d676ce1

Update test/e2e/launch.e2e.test.js

8e3d4e7

Update test/e2e/launch.e2e.test.js

aec2293

SgtPooki marked this pull request as ready for review July 8, 2023 02:17

SgtPooki requested review from a team and whizzzkid as code owners July 8, 2023 02:17

whizzzkid reviewed Jul 8, 2023

View reviewed changes

Base automatically changed from feat/ctx-refactor to main July 18, 2023 22:51

SgtPooki added 6 commits July 31, 2023 12:21

Merge branch 'main' into feat/ctx-refactor-splash-screen

7b8af6a

fix: do not prompt for new port when using CI

4155efd

fix: handle errors that were breaking e2e tests

09d9d10

feat: splashscreen page background is transparent

497bdc2

* make keyframe animation dir normal * remove visual artifacts of drop-shadow on size reduction * make drop-shadow animation better

feat: splash screen animation polish

dfcabe2

chore: removed webkit keyframes definition

0d550bc

SgtPooki commented Aug 1, 2023

View reviewed changes

chore: remove debugging code

6ede66f

SgtPooki requested a review from whizzzkid August 1, 2023 01:04

whizzzkid approved these changes Aug 1, 2023

View reviewed changes

SgtPooki and others added 2 commits August 1, 2023 11:46

chore: remove extra line

bf58346

Co-authored-by: Nishant Arora <[email protected]>

chore(splash-screen): repositioning awaits

4277da6

Co-authored-by: Nishant Arora <[email protected]>

SgtPooki commented Aug 1, 2023

View reviewed changes

SgtPooki added 2 commits August 1, 2023 14:49

chore: fix after pr comment change

b3b4e49

chore: move splash screen css to separate file

7819ed2

SgtPooki merged commit 417875d into main Aug 7, 2023
8 checks passed

SgtPooki deleted the feat/ctx-refactor-splash-screen branch August 7, 2023 14:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add splash screen on top of ctx refactor #2548

feat: add splash screen on top of ctx refactor #2548

SgtPooki commented Jul 8, 2023 •

edited

Loading

SgtPooki left a comment

SgtPooki Jul 8, 2023

whizzzkid Jul 8, 2023

SgtPooki Jul 31, 2023

whizzzkid left a comment

whizzzkid Jul 8, 2023

whizzzkid Jul 8, 2023

SgtPooki left a comment

SgtPooki Jul 31, 2023

SgtPooki Aug 1, 2023

SgtPooki Aug 1, 2023

SgtPooki Aug 1, 2023

SgtPooki Aug 1, 2023

SgtPooki Aug 1, 2023

whizzzkid Aug 1, 2023

SgtPooki Aug 1, 2023

whizzzkid left a comment

whizzzkid Aug 1, 2023

whizzzkid Aug 1, 2023

SgtPooki Aug 1, 2023

SgtPooki commented Aug 1, 2023

		await splashScreen.loadFile(path.join(__dirname, './splash.html'))
		splashScreen.center()

		@@ -0,0 +1 @@
		export type CONFIG_KEYS = 'AUTO_LAUNCH' \| 'AUTO_GARBAGE_COLLECTOR' \| 'SCREENSHOT_SHORTCUT' \| 'OPEN_WEBUI_LAUNCH' \| 'MONOCHROME_TRAY_ICON' \| 'EXPERIMENT_PUBSUB' \| 'EXPERIMENT_PUBSUB_NAMESYS'

feat: add splash screen on top of ctx refactor #2548

feat: add splash screen on top of ctx refactor #2548

Conversation

SgtPooki commented Jul 8, 2023 • edited Loading

Demo

SgtPooki 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

whizzzkid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SgtPooki 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

whizzzkid 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

SgtPooki commented Aug 1, 2023

SgtPooki commented Jul 8, 2023 •

edited

Loading