Refactor the Extensions settings page #2221

Nokel81 · 2021-02-24T21:34:43Z

Simplify the install logic by refactoring out the multi-install logic
Revamp the ExtensionInstallStateStore to more strictly track the lifetime of an install or uninstall request
Fix the install of an already installed extension just hanging visually
Display a spinner more often for more visual feedback

Signed-off-by: Sebastian Malton sebastian@malton.name

fixes #2116

Nokel81 · 2021-02-25T01:58:40Z

twice-install.mov

github-actions · 2021-03-08T13:17:37Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2021-03-08T15:15:39Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions · 2021-03-15T12:59:39Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2021-03-15T14:18:28Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

aleksfront · 2021-03-29T09:55:19Z

While it works okay in case of twice install, but I found a few issues:

It doesn't clear disabled state if wrong link provided
When installed initial extension from a tar file, there's no loading state showed (not inside Install button nor with Spinner at the bottom).
It doesn't clear disabled state if close button clicked inside notification. Spinner is spinning and input keeps disabled.
When installing same extension twice, green notification about successful uninstall appears. Than nothing happens - no any loading state (but new extension getting installed in background).
Why we replaced "Install state" visualisation from button? I think we don't need to show Spinner instead when installation is in progress. At least we need to keep both.

src/renderer/components/+extensions/extension-install.store.ts

jim-docker · 2021-03-29T16:48:24Z

Why we replaced "Install state" visualisation from button? I think we don't need to show Spinner instead when installation is in progress. At least we need to keep both.

If you already have a long list of extensions the spinner is not even visible unless you scroll to the bottom

Nokel81 · 2021-03-29T16:49:29Z

I see know, reverting to the button having the animation.

Nokel81 · 2021-03-29T17:44:59Z

@aleksfront @jakolehm @jim-docker I have fixed the issues that Aleks discovered and reverted to the older loading animation (for install). PTALA

aleksfront · 2021-03-30T07:59:33Z

@Nokel81 There's still 2 issue available. 1 and 3 from the previous list.

It shows [object Object] error message and doesn't drop loading state.
It doesn't clear disabled state if close button clicked inside notification.

github-actions · 2021-03-30T09:08:09Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Add support for installing extensions by name - Optionally specify the version, defaults to latest non-prerelease version - Usable via the protocol handlers, but requires user confirmation to actually install Signed-off-by: Sebastian Malton <sebastian@malton.name> * resolve pr comments Signed-off-by: Sebastian Malton <sebastian@malton.name> * add confirm dialog message Signed-off-by: Sebastian Malton <sebastian@malton.name> * display error when registry returns not found as JSON Signed-off-by: Sebastian Malton <sebastian@malton.name> * fix unit test Signed-off-by: Sebastian Malton <sebastian@malton.name> * fix registry error detection Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/extensions/extension-discovery.ts

jim-docker · 2021-04-13T19:27:38Z

src/extensions/extension-loader.ts

-      if (extEntrypoint !== "") {
-        if (!fs.existsSync(extEntrypoint)) {
-          console.log(`${logModule}: entrypoint ${extEntrypoint} not found, skipping ...`);
+    const extensionEntryPointAbsolutePath = path.resolve(path.join(path.dirname(extension.manifestPath), extensionEntryPointRelativePath));


these path functions can all throw a TypeError so why not keep this statement in the try block?

They can throw but only if it is not a string.

jim-docker · 2021-04-13T19:39:48Z

src/renderer/components/+extensions/__tests__/extensions.test.tsx


-    fireEvent.change(screen.getByPlaceholderText("Path or URL to an extension package", {
+    fireEvent.change(res.getByPlaceholderText("Path or URL to an extension package", {


The placeholder text has changed but I guess this still works because it's not looking for an exact match?

src/renderer/components/+extensions/extension-install.store.ts

src/renderer/components/+extensions/extensions.tsx

Signed-off-by: Sebastian Malton <sebastian@malton.name>

jim-docker · 2021-04-14T20:59:02Z

src/renderer/components/+extensions/extensions.tsx

-          dispose();
-          fse.unlink(validatedRequest.tempFile).catch(noop);
-        }
+        onClose: dispose,


much cleaner!

jakolehm

LGTM

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix: logs data disapearing causing crashes (#2566) Signed-off-by: Sebastian Malton <sebastian@malton.name> * Refactor helm-chart.api and improve kube validation and error handling (#2265) Signed-off-by: Sebastian Malton <sebastian@malton.name> * Fix: HPA's not sortable by age (#2565) Signed-off-by: Sebastian Malton <sebastian@malton.name> * Conditionally render status icon for kube meta (#2298) Signed-off-by: Sebastian Malton <sebastian@malton.name> * Fix custom resource loading spinner appears above extensions' cluster menus (#2344) Signed-off-by: Sebastian Malton <sebastian@malton.name> * Lens should point to the release docs (#2268) Signed-off-by: Sebastian Malton <sebastian@malton.name> * Refactor the Extensions settings page (#2221) Signed-off-by: Sebastian Malton <sebastian@malton.name> * try and get jest to not core dump Signed-off-by: Sebastian Malton <sebastian@malton.name>

Nokel81 added bug Something isn't working area/ui area/extension Something to related to the extension api labels Feb 24, 2021

Nokel81 requested a review from a team February 24, 2021 21:34

Nokel81 self-assigned this Feb 24, 2021

github-actions bot added the PR: needs rebase label Mar 8, 2021

Nokel81 force-pushed the twice-install branch from d3b17e4 to ddae241 Compare March 8, 2021 15:15

github-actions bot removed the PR: needs rebase label Mar 8, 2021

github-actions bot added the PR: needs rebase label Mar 15, 2021

Nokel81 force-pushed the twice-install branch from ddae241 to 3fabaf1 Compare March 15, 2021 14:18

github-actions bot removed the PR: needs rebase label Mar 15, 2021

Nokel81 force-pushed the twice-install branch from 3fabaf1 to edad6d5 Compare March 16, 2021 14:04

Nokel81 added this to the 4.2.1 milestone Mar 24, 2021

Nokel81 mentioned this pull request Mar 29, 2021

Add support for registry.npmjs.com URLs in the extension install input #2394

Closed

jakolehm reviewed Mar 29, 2021

View reviewed changes

src/renderer/components/+extensions/extension-install.store.ts Show resolved Hide resolved

jakolehm previously approved these changes Mar 29, 2021

View reviewed changes

Nokel81 dismissed jakolehm’s stale review via f4265f7 March 29, 2021 17:44

Nokel81 requested a review from a team March 29, 2021 17:44

github-actions bot added the PR: needs rebase label Mar 30, 2021

Nokel81 added the merge/master This PR will need to be merged to master after merged to target. Using cherry-pick. label Apr 6, 2021

Nokel81 added 3 commits April 6, 2021 17:19

some fixes to unit tests

6c6d719

Signed-off-by: Sebastian Malton <sebastian@malton.name>

fix unit tests

5e69520

Signed-off-by: Sebastian Malton <sebastian@malton.name>

remove flaky test

ed81d8f

Signed-off-by: Sebastian Malton <sebastian@malton.name>

Nokel81 modified the milestones: 4.2.1, 4.2.2 Apr 9, 2021

Nokel81 requested review from jakolehm, a team and jim-docker and removed request for a team April 13, 2021 12:09

jim-docker reviewed Apr 13, 2021

View reviewed changes

respond to review comments

9550f07

Signed-off-by: Sebastian Malton <sebastian@malton.name>

Nokel81 requested a review from jim-docker April 14, 2021 12:38

jim-docker approved these changes Apr 14, 2021

View reviewed changes

better use of disposers

865e893

Signed-off-by: Sebastian Malton <sebastian@malton.name>

jim-docker reviewed Apr 14, 2021

View reviewed changes

Nokel81 modified the milestones: 4.2.2, 4.2.3 Apr 16, 2021

Nokel81 added the blocker label Apr 19, 2021

jakolehm approved these changes Apr 19, 2021

View reviewed changes

Nokel81 merged commit 86205b3 into release/v4.2 Apr 19, 2021

Nokel81 deleted the twice-install branch April 19, 2021 17:50

Nokel81 mentioned this pull request Apr 26, 2021

release v4.2.3 #2627

Merged

Nokel81 added a commit that referenced this pull request Apr 26, 2021

Refactor the Extensions settings page (#2221)

bd5d227

Nokel81 added a commit that referenced this pull request Apr 26, 2021

Refactor the Extensions settings page (#2221)

e474246

Nokel81 added a commit that referenced this pull request Apr 26, 2021

Refactor the Extensions settings page (#2221)

c3dcf0e

Signed-off-by: Sebastian Malton <sebastian@malton.name>

Nokel81 mentioned this pull request May 19, 2021

Restyling extensions page with tailwindcss #2796

Merged

Nokel81 removed the merge/master This PR will need to be merged to master after merged to target. Using cherry-pick. label Jun 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the Extensions settings page #2221

Refactor the Extensions settings page #2221

Nokel81 commented Feb 24, 2021

Nokel81 commented Feb 25, 2021

github-actions bot commented Mar 8, 2021

github-actions bot commented Mar 8, 2021

github-actions bot commented Mar 15, 2021

github-actions bot commented Mar 15, 2021

aleksfront commented Mar 29, 2021 •

edited

jim-docker commented Mar 29, 2021

Nokel81 commented Mar 29, 2021

Nokel81 commented Mar 29, 2021

aleksfront commented Mar 30, 2021

github-actions bot commented Mar 30, 2021

jim-docker Apr 13, 2021

Nokel81 Apr 14, 2021

jim-docker Apr 13, 2021

Nokel81 Apr 14, 2021

jim-docker Apr 14, 2021

jakolehm left a comment


		fireEvent.change(screen.getByPlaceholderText("Path or URL to an extension package", {
		fireEvent.change(res.getByPlaceholderText("Path or URL to an extension package", {

Refactor the Extensions settings page #2221

Refactor the Extensions settings page #2221

Conversation

Nokel81 commented Feb 24, 2021

Nokel81 commented Feb 25, 2021

github-actions bot commented Mar 8, 2021

github-actions bot commented Mar 8, 2021

github-actions bot commented Mar 15, 2021

github-actions bot commented Mar 15, 2021

aleksfront commented Mar 29, 2021 • edited

jim-docker commented Mar 29, 2021

Nokel81 commented Mar 29, 2021

Nokel81 commented Mar 29, 2021

aleksfront commented Mar 30, 2021

github-actions bot commented Mar 30, 2021

jim-docker Apr 13, 2021

Choose a reason for hiding this comment

Nokel81 Apr 14, 2021

Choose a reason for hiding this comment

jim-docker Apr 13, 2021

Choose a reason for hiding this comment

Nokel81 Apr 14, 2021

Choose a reason for hiding this comment

jim-docker Apr 14, 2021

Choose a reason for hiding this comment

jakolehm left a comment

Choose a reason for hiding this comment

aleksfront commented Mar 29, 2021 •

edited