Skip to content

feat: publish extensions overhaul - #2098

Open
gnugomez wants to merge 8 commits into
eclipse-openvsx:mainfrom
gnugomez:gnugomez/main/publish-extension-page
Open

feat: publish extensions overhaul#2098
gnugomez wants to merge 8 commits into
eclipse-openvsx:mainfrom
gnugomez:gnugomez/main/publish-extension-page

Conversation

@gnugomez

Copy link
Copy Markdown
Member

Replaces the one-file-at-a-time publish dialog with a /publish page and a drag-and-drop flow. Dragging a file anywhere turns the navbar's Publish button into a drop area; everything dropped on it is uploaded straight away, no confirmation. The queue lives in a context, so it survives navigation, and renders as a line of extension cards that poll until each package settles.

The button is exported as PublishButton — it carries the link, the p shortcut and the drop target together, so a deployment with its own menu content keeps all three.

Screen.Recording.2026-08-25.at.13.07.46.mov

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR overhauls extension publishing in the Web UI by replacing the per-extension publish dialog with a dedicated /publish page and a global drag-and-drop publishing flow backed by a persistent publish queue context.

Changes:

  • Add a /publish page with a multi-file picker + drop area that immediately enqueues/uploads .vsix packages and shows progress as an inline card strip.
  • Introduce an app-wide publish queue context with polling to reflect post-upload outcomes (review verdicts, icon availability).
  • Turn the navbar “Publish” control into an exported PublishButton that combines link + p shortcut + drop target.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
webui/test/unit/pages/publish/publish-page.spec.tsx New unit tests covering /publish page upload/drop/login states and queue visibility.
webui/test/unit/context/publish-queue-context.spec.tsx New unit tests for publish queue behavior, error handling, namespace creation, and polling.
webui/test/unit/components/publish/publish-queue-strip.spec.tsx New unit tests for the inline queue strip rendering and states.
webui/test/unit/components/publish/publish-button.spec.tsx New unit tests for the navbar publish button link/shortcut/drop behavior.
webui/src/utils.ts Add formatFileSize() helper for human-readable byte formatting.
webui/src/pages/user/extensions/user-settings-extensions.tsx Replace old publish dialog action with a link button to /publish.
webui/src/pages/user/extensions/publish-extension-dialog.tsx Remove legacy one-file-at-a-time publish dialog implementation.
webui/src/pages/publish/publish-routes.ts Introduce publish route constants (PublishRoutes.ROOT).
webui/src/pages/publish/publish-page.tsx Implement new publish page UI (drop area, file picker, command-line alternative, inline queue).
webui/src/layout/app-layout.tsx Register the new /publish route.
webui/src/index.ts Export PublishButton for custom deployments/menus.
webui/src/extension-registry-service.ts Make icon access resilient (files?.icon) in getExtensionIcon flow.
webui/src/default/menu-content.tsx Replace old publish shortcut/button with PublishButton; mobile menu links to /publish.
webui/src/context/publish-queue-context.tsx New publish queue provider: enqueue uploads, retry with namespace creation, poll for review/icon settling.
webui/src/components/publish/use-publish-drop.ts New hook for window-level drag detection + drop target props + navigation to /publish.
webui/src/components/publish/publish-queue-strip.tsx New horizontal card strip rendering queue items with status/clear UX and accept “flash”.
webui/src/components/publish/publish-button.tsx New navbar publish button component (link + shortcut + drop target).
webui/src/components/extension/use-extension-icon.ts Avoid crashing when files is missing (files?.icon) in query key.
webui/src/components/extension/manage-extension-card.tsx Add support for iconPending and custom footer content (used by publish queue).
webui/src/components/extension/extension-icon.tsx Add pending prop to keep skeleton visible until icon exists.
webui/src/components/extension-card.tsx Thread through iconPending to ExtensionIcon.
webui/src/app-providers.tsx Add PublishQueueProvider to the app provider stack so queue survives navigation.
webui/CHANGELOG.md Document new publishing page/flow and PublishButton export.
Suppressed comments (3)

webui/test/unit/context/publish-queue-context.spec.tsx:102

  • publishExtension should resolve to an Extension, not an array. Using [published()] here can cause the publish queue to poll for up to 60s (because files.icon is missing on an array), making this test slow/flaky.
    it('creates the namespace when the error comes back as a value instead of a throw', async () => {
        const publishExtension = vi
            .fn()
            .mockResolvedValueOnce({ error: 'Unknown publisher: foo\nUse the CLI to create it' })
            .mockResolvedValueOnce([published()]);
        const createNamespace = vi.fn().mockResolvedValue({ success: 'ok' });

webui/test/unit/context/publish-queue-context.spec.tsx:116

  • Same issue as above: publishExtension should resolve to a single Extension (or reject with ErrorResult), not [Extension]. Returning an array here can trigger the queue’s polling loop and make the test hang/flap.
    it('reads the namespace even when the message carries no second line', async () => {
        const publishExtension = vi
            .fn()
            .mockRejectedValueOnce({ error: 'Unknown publisher: foo' })
            .mockResolvedValueOnce([published()]);
        const createNamespace = vi.fn().mockResolvedValue({ success: 'ok' });

webui/test/unit/context/publish-queue-context.spec.tsx:152

  • publishExtension resolves to an Extension, not an array. Returning [published()] here can keep the item in awaitingIcon polling for up to 60s, making the test unnecessarily slow/flaky.
        const publishExtension = vi
            .fn()
            .mockResolvedValueOnce([published()])
            .mockReturnValueOnce(new Promise(() => {}));

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread webui/test/unit/context/publish-queue-context.spec.tsx
Comment thread webui/src/context/publish-queue-context.tsx Outdated
Comment thread webui/src/context/publish-queue-context.tsx
Comment thread webui/src/components/publish/use-publish-drop.ts

@netomi netomi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice feature — reviewed the queue/poll logic and the drop-target wiring in detail. Found one feature-breaking gap and a few real logic bugs in the poll/queue state machine, plus some lower-priority efficiency and duplication notes. All confirmed by reading the code directly (not just tool output). Inline comments below; a couple more that don't anchor to one line:

  • No client-side file-size check before upload (a regression from the deleted publish-extension-dialog.tsx, which used react-dropzone's maxSize to reject oversized files instantly). publish() (publish-queue-context.tsx:268) only filters on isVsixFile; an oversized file now fully uploads before the server rejects it.
  • Every concurrent upload polls independently — each pollUntilSettled call hits the full extensions-list endpoint on its own 5s timer, so N concurrent items means N× redundant full-list GETs every tick instead of one shared read.
  • No upload concurrency cappublish()'s queued.forEach(({ id, file }) => upload(id, file)) (publish-queue-context.tsx:279) fires every queued file's upload simultaneously with no batching/throttling.
  • Smaller/lower-priority, not blocking: statusOf() duplicates getExtensionStatus's precedence logic (risk of divergence); errorMessage() diverges from utils.ts's handleError (drops the .message half of combined error objects); the hand-rolled drag-depth tracking in use-publish-drop.ts reimplements what react-dropzone already did — and that package is now an unused dependency left in package.json; dismiss() is exported/wired but never called from any UI; the new formatFileSize util has no test coverage; the new iconPending prop on ExtensionCardProps isn't reflected in the existing unreleased CHANGELOG bullet for that interface.

{loginProviders && !location.pathname.startsWith(UserSettingsRoutes.ROOT) && (
<MenuItem component={RouteLink} to={UserSettingsRoutes.EXTENSIONS}>
{loginProviders && !location.pathname.startsWith(PublishRoutes.ROOT) && (
<MenuItem component={RouteLink} to={PublishRoutes.ROOT}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drag-and-drop publishing doesn't work on mobile at all: usePublishDrop() — the hook that attaches the window-level drag listeners — is only called inside PublishButton (publish-button.tsx). MobileMenuContent renders a plain MenuItem here instead of <PublishButton />, so on any viewport below the lg breakpoint, dragging a .vsix file anywhere falls through to the browser's native handling — exactly the failure PublishButton's own doc comment warns must be avoided ("or publishing by drag and drop has nowhere to land"). Worth rendering PublishButton here too, or at least wiring usePublishDrop() into the mobile shell some other way.

@gnugomez gnugomez Aug 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real gap, but PublishButton here would not close it: the mobile Menu is unmounted while closed, so the listeners would only be armed with the menu open. Leaving drag-and-drop as a desktop affordance; the menu item still links to /publish.

))}
{loginProviders && !location.pathname.startsWith(UserSettingsRoutes.ROOT) && (
<MenuItem component={RouteLink} to={UserSettingsRoutes.EXTENSIONS}>
{loginProviders && !location.pathname.startsWith(PublishRoutes.ROOT) && (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separately: this condition narrowed from !location.pathname.startsWith(UserSettingsRoutes.ROOT) (suppressed on all /user-settings/* pages) to !location.pathname.startsWith(PublishRoutes.ROOT) (suppressed only on /publish). Since /user-settings/extensions now has its own "Publish extension" entry point per this PR, mobile users visiting that page will see this menu item and that button — the old condition avoided exactly this duplication.

@gnugomez gnugomez Aug 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The item now links to /publish, so the condition keeps the same rule: do not link to the page you are on. A nav item plus a page action is the pairing the desktop nav already has.

export const PublishButton: FunctionComponent<PublishButtonProps> = ({ sx, className }) => {
const navigate = useNavigate();
const { dragging, over, dropProps } = usePublishDrop();
useShortcut({ key: 'p', label: 'Publish', order: 3, callback: () => navigate(PublishRoutes.ROOT) });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The p shortcut lost its enabled guard here. The previous registration (in menu-content.tsx on main) had enabled: !!loginProviders. PublishButton is exported from webui/src/index.ts specifically for third-party deployments to embed directly ("render this rather than its own button"), so a consumer that mounts <PublishButton /> unconditionally now gets an always-active global p shortcut with no prop to disable it.

@gnugomez gnugomez Aug 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the default menu the button only renders under loginProviders, so the gating is unchanged. For an embedder, mounting PublishButton is the opt-in; an enabled prop no deployment has asked for would be a speculative seam.

Comment thread webui/src/context/publish-queue-context.tsx Outdated
Comment thread webui/src/context/publish-queue-context.tsx
Comment thread webui/src/context/publish-queue-context.tsx
Comment thread webui/src/context/publish-queue-context.tsx Outdated
@gnugomez

Copy link
Copy Markdown
Member Author

I will migrate the publish to tanstack as well, just to avoid the abort that doesn't make a lot of sense here

@gnugomez
gnugomez force-pushed the gnugomez/main/publish-extension-page branch from b58f605 to a960230 Compare August 28, 2026 11:00
@gnugomez
gnugomez requested a lite review from Copilot August 28, 2026 11:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.

Comment thread webui/src/pages/user/namespaces/create-namespace-dialog.tsx
@gnugomez
gnugomez requested a review from netomi August 28, 2026 11:57
@gnugomez
gnugomez force-pushed the gnugomez/main/publish-extension-page branch from 8867460 to 8c6b3c9 Compare August 28, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants