Skip to content

Add Refresh and Back to the onboarding "No workspaces yet" screen - #2216

Open
subodhr258 wants to merge 6 commits into
developfrom
fix/onboarding-no-workspaces-dead-end
Open

subodhr258 wants to merge 6 commits into
developfrom
fix/onboarding-no-workspaces-dead-end

Conversation

@subodhr258

@subodhr258 subodhr258 commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #2221

Review packet

In one line: The onboarding screen "No workspaces yet" now has Refresh and Back, so a new user who creates a workspace in the GoDAM app can carry on without reloading wp-admin and signing in again. Fixes #2221.

1. Criteria and proof

Acceptance criterion Proof
With no workspace on the account, Refresh sends 1 request, stays on the screen and says "Still no workspaces on this account…" Test "Refresh that still finds nothing stays put and says so"; recording V2 step 4; still S02
Once a workspace exists, Refresh opens the workspace picker with that workspace selected and Continue enabled, with 1 sign-in in total (develop needed 2 sign-ins and a reload) Test "Refresh moves on to the picker, with the new workspace already selected"; V2 step 6 (right pane: 1 sign-in, 3 list requests); V1 for the 2 sign-ins; still S03
Back leaves the screen for the first onboarding screen Test "Back returns to the first screen"; on the site, after Back a second sign-in reached the picker
A failed Refresh keeps the screen and shows the service's message Test "a failed Refresh shows the server message and keeps the screen"; on the site with the service answering 401, the message "Your session has expired. Please sign in again." showed and Refresh stayed usable
A Refresh answer that arrives after Back is ignored Test "an answer that arrives after Back does not touch the first screen"

2. Before and after

Both recordings run the same flow on the same site (godam-dev) with the sign-in and workspace-list calls of the GoDAM service stubbed in the browser, so nothing is sent to app.godam.io. Left pane: the site admin in wp-admin. Right pane: the stubbed GoDAM service, showing the workspaces on the account and every onboarding request the page sent. The email and password typed are placeholders that only reach the stub.

Before (develop): no way forward. The screen has no Refresh, Back or Close. After the workspace is created the screen does not ask again, the only way on is to reload wp-admin, which starts the onboarding over, and the admin signs in a second time.

V1 develop: no way forward from No workspaces yet

B1 develop: the screen has only the link to the GoDAM app

After (this branch): Refresh lands on the workspace picker. Refresh before a workspace exists says so and stays. After the workspace is created, Refresh opens the picker with it selected: no reload, no second sign-in.

V2 branch: Refresh opens the workspace picker

S01 the empty screen now has Back and Refresh

S02 Refresh before a workspace exists

S03 Refresh opens the workspace picker with the new workspace selected

Also checked on the same site, not filmed: a failed Refresh, a double click sends 1 request, the button reads "Checking…" and is disabled while loading, the Contact support link opens in a new tab, and at 390 px wide Back and Refresh stay inside the screen.

3. Tests

  • Added: pages/onboarding/components/screens/WorkspaceScreen.test.js (8 tests). They mount the screen with the real store and slice and mock only the two request hooks. They assert: the empty screen shows Back, Refresh, the create-workspace link and the support link; Refresh finding a workspace opens the picker with it selected and Continue enabled; Refresh finding nothing sets an info notice and stays; a failed Refresh sets the error notice with the service's message; a successful Refresh clears an earlier notice; the button reads "Checking…" and is disabled while loading; a late answer after Back leaves the first screen untouched; Back returns to the first screen.
  • Changed: none.
  • Revert check: against the screen as it is on develop, the 6 tests written first all fail because the buttons do not exist ("Cannot read properties of null (reading 'dispatchEvent')"). With only the "selected workspace follows the list" part reverted, "Refresh moves on to the picker…" fails with Expected substring: "Acme Videos", Received string: "▾" (an empty selection). With only the late-answer guard reverted, "an answer that arrives after Back…" fails because the "Still no workspaces…" notice appears on the first screen.

4. Risk flags

No risk flags fired on #2216. Nothing in the diff touched the watched areas.

5. Independent review

A separate review pass saw only the task and the diff. Its findings, checked against the code:

Finding Grade What I did
WorkspaceScreen.jsx handleRefresh: a Refresh answer that arrives after Back writes its notice into the reset state Valid, low Fixed in 1842668: the screen ignores an answer that arrives after it has gone; test added
languages/godam.pot was stale, so the translation-template check fails Valid Fixed in 96ac130 and again in 2c5ee84 by rebuilding with build:prod and composer pot. The file is no longer part of this PR: develop now runs the POT check only on PRs into main (#2188)
The Contact support URL is the production helpdesk and may need an app login Overstated No change: it is the same link the admin pages use, it opens in a new tab, and the person on this screen has a GoDAM account
Refresh is disabled while loading, and the button unmounts on success, so keyboard focus can drop to the page Valid, low, not new No change: Continue and Sign in do the same on every step of this onboarding
A second identical "Still no workspaces…" answer shows no new message Valid, low No change: the button's "Checking…" state is the visible feedback

Review comment from KMchaudhary (LGTM, two polish notes): the redundant clearNotice() on the empty path was removed in af8458c. The focus note describes the same pattern Continue and Sign in already have across this onboarding, so no change here.

6. Not verified

  • A real GoDAM account with a real new workspace: the service was stubbed, so the real list_my_organizations answer, and whether the service caches an empty list for a short time, were not exercised.
  • The expired-session path with a real expired token (it was checked with the service answering 401, not by waiting for the real 24-hour expiry).
  • The GoDAM Screens canvas (row 1) has no board for this screen, so there was no design to match. The Back link and button styles are the ones the other onboarding screens already use.

7. Scope

One string reworded: the empty-state sentence now ends "then come back here and select Refresh." Added a Contact support link on the same screen, and SUPPORT_URL in pages/onboarding/utils/constants.js. The picker now defaults to the first workspace of the current list, which is what lets it open with a workspace selected after a Refresh. Nothing else.

An account with no workspaces landed on a screen whose only control was the
"create one" link. After creating a workspace in the GoDAM app the user had
to reload wp-admin and sign in again, because a reload resets the onboarding
to its first screen.

The screen now has:
- Refresh: asks for the workspace list again (the sign-in session is still
  held server-side) and moves to the workspace picker once one exists. If
  nothing is found yet it says so; if the request fails it shows the server
  message.
- Back: returns to the first screen, same as the close button on the picker.
- A Contact support link.

The picker now defaults to the first workspace in the list as it stands, so it
opens with a workspace selected after a Refresh finds the first one.
…e screen

Back leaves the screen while a Refresh may still be waiting on the GoDAM
service. The late answer used to set its notice on the first screen the user
had gone back to. The screen now drops an answer that arrives after it has
gone, and the tests cover that case and the Checking state.
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
15 0 15

⚠️ Warnings (15)

📁 inc/classes/class-video-sitemap.php (1 warning)
📍 Line 🔖 Check 💬 Message
620 WordPress.DB.SlowDBQuery.slow_db_query_meta_key Detected usage of meta_key, possible slow query.
📁 readme.txt (2 warnings)
📍 Line 🔖 Check 💬 Message
0 mismatched_plugin_name Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM".
0 trademarked_term The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name.
📁 inc/classes/class-video-sitemap-core-provider.php (1 warning)
📍 Line 🔖 Check 💬 Message
73 WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "wp_sitemaps_index_entry".
📁 assets/build/css/main.css (1 warning)
📍 Line 🔖 Check 💬 Message
0 EnqueuedStylesScope This style is being loaded in all contexts.
📁 assets/src/libs/analytics.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/20/hello-world/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
📁 assets/build/js/main.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/20/hello-world/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

Regenerated with a production build and `composer pot`: the new Refresh,
Checking…, Contact support and "Still no workspaces" strings, the reworded
empty-state sentence, and the moved source references.
@subodhr258
subodhr258 marked this pull request as ready for review October 19, 2026 11:01
@KMchaudhary

Copy link
Copy Markdown
Collaborator

Code review

Verdict: LGTM — a clean, well-tested UX fix. The two load-bearing bits are both right:

  • const selected = chosen || organizations[0] || null; — deriving the selection (instead of the old one-time useState(organizations[0])) is exactly what makes a Refresh-discovered workspace auto-select, since it now reacts to organizations changing. I checked the picker branch: selected is read-only there and setSelected sets chosen, so every existing usage (dropdown toggle, checkmark, handleContinue, Continue's disabled) stays correct.
  • The isMounted guard — Back is not disabled during a refresh, so a user can go Back mid-request; reset() unmounts the screen and the late unwrap() resolution is dropped by the isMounted.current check before any dispatch. Nicely covered by the "answer that arrives after Back" test.

I also confirmed the dependencies this leans on all exist: listOrganizations mutation (useListOrganizationsMutation), clearNotice/setOrganizations in the slice, and BackButton carrying data-test-id="godam-onboarding-button-back". Error handling degrades cleanly (error?.data?.message || fallback), and { organizations: latest = [] } guards a missing field.

Two polish-level notes, neither blocking:

1. Redundant clearNotice() on the empty path — WorkspaceScreen.jsx handleRefresh

When a Refresh still finds nothing, you dispatch( clearNotice() ) and then immediately dispatch( setNotice({ status: 'info', … }) ) in the same tick, so the clear is overwritten right away. You could clear only on the non-empty branch. Purely cosmetic.

2. Focus drops when Refresh unmounts — WorkspaceScreen.jsx

Refresh is disabled while loading and the empty screen unmounts on success, so a keyboard user lands on document.body. As you note, Continue/Sign in do the same across this flow, so it's not new — just flagging it as a good candidate for a one-time focus-management pass over the onboarding screens later.

Nice test design — mounting the real store + slice and mocking only the RTK hooks, plus the revert check proving each assertion bites.

🤖 Automated review via Claude Code (/code-review, high effort).

When Refresh still finds none, the notice was cleared and set again in the
same tick; now only the info notice is set.

Addresses review: #2216 (comment)
@subodhr258

Copy link
Copy Markdown
Collaborator Author

@KMchaudhary on your comment:

Thanks Kuldip.

  1. Redundant clearNotice(): fixed in af8458c. The notice is now cleared only when Refresh finds a workspace, and the 8 WorkspaceScreen tests still pass.
  2. Focus after Refresh: agreed, it's the same across the onboarding screens, so it goes on the list for a focus pass over that flow later. No change here.

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.

2 participants