Skip to content

fix(bookmark): show app name loading state - #5111

Merged
odinr merged 3 commits into
equinor:mainfrom
hugosmoreira:agent/fix-edit-bookmark-loading-skeleton
Jul 30, 2026
Merged

fix(bookmark): show app name loading state#5111
odinr merged 3 commits into
equinor:mainfrom
hugosmoreira:agent/fix-edit-bookmark-loading-skeleton

Conversation

@hugosmoreira

Copy link
Copy Markdown
Contributor

Why is this change needed?

The Edit Bookmark modal leaves the read-only app name field blank while the app manifest is being resolved. That makes a normal asynchronous wait look like missing data.

What is the current behavior?

The modal requests the app manifest and eventually fills in its display name, but it provides no loading feedback while that request is pending.

What is the new behavior?

The app name field now displays an EDS circular progress indicator while the manifest request is pending. The resolved display name replaces it when available. The field also exposes its busy state and an accessible loading label.

What is the intended behavior or invariant?

Loading is reported only after both the bookmark and app provider exist, while the manifest value is unresolved and no request error has occurred. A missing provider or failed request must not leave the field permanently busy.

Does this PR introduce a breaking change?

No.

Impact assessment:

  • Breaking changes: No
  • Version bump: Patch
  • Consumer impact: The Edit Bookmark modal gains visual and accessible loading feedback; no API changes are required.
  • Downstream impact: Limited to @equinor/fusion-framework-react-components-bookmark.

Review guidance:

Please focus on the pending-state condition in EditBookmark.tsx and the loading/resolved rendering contract in AppNameField.

Validation completed:

  • focused package tests: 2 passed
  • complete repository test suite: 585 passed
  • complete repository build: 80 tasks passed
  • affected package build: passed
  • scoped Biome and Fusion lint checks: passed with no new warnings
  • manual local render: loading and resolved states verified, including aria-busy and the progress label

Additional context

The implementation uses the existing EDS Input adornment and Progress.Circular components, so it adds no runtime dependency. A patch changeset and package README update are included.

The repository-wide pnpm -w check could not be used as a clean signal on this Windows checkout because it reports existing CRLF formatting across untouched files and a Biome configuration version mismatch. The changed TypeScript files pass a scoped Biome check and were not used to reformat unrelated files.

Related issues

closes: #5092

Checklist

  • Confirm completion of the self-review checklist
  • Confirm TSDoc captures intent for functions, hooks, components, classes, and named arrow functions
  • Confirm iterator blocks, decision gates, RxJS chains, and complex decisions explain why they exist
  • Confirm React logic and derived values are resolved before markup when applicable
  • Confirm README/docs are updated for user-facing changes
  • Confirm changes to target branch validation
    • Included files validated
    • No new linting warnings
    • Not a duplicate PR (check existing)
  • Confirm adherence to code of conduct

@changeset-bot

changeset-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 13c2ed8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@equinor/fusion-framework-react-components-bookmark Patch
@equinor/fusion-framework-dev-portal Patch
@equinor/fusion-framework-cli Patch
poc-portal Patch
portal-analytics Patch
portal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added 🐞 bug Something isn't working 👾 React 📚 documentation Improvements or additions to documentation labels Jul 27, 2026
@hugosmoreira
hugosmoreira force-pushed the agent/fix-edit-bookmark-loading-skeleton branch from 4472530 to 40a2ed4 Compare July 29, 2026 14:31
@odinr
odinr force-pushed the agent/fix-edit-bookmark-loading-skeleton branch from 40a2ed4 to 5bf4aea Compare July 30, 2026 07:07
@github-actions github-actions Bot added the 🚧 chore maintaines work, (update deps, workflos ...) label Jul 30, 2026
@odinr

odinr commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

🔍 Security review — external contributor

Since this PR comes from an unfamiliar external contributor (fork PR), I ran an extensive supply-chain and code-level security check before doing anything else.

Contributor signal

  • GitHub account hugosmoreira: active since 2018, 195 public repos, 23 followers — established account, not a throwaway/bot profile.
  • No claimed affiliation with Equinor/Fusion, but the repo is open source, so external contributions are expected and welcome.

Diff scope (9 files, +93/-7)

File Risk assessment
.changeset/react-components-bookmark_app-loading.md Standard changeset, patch bump, no code.
packages/react/components/bookmark/README.md Doc-only line change.
packages/react/components/bookmark/package.json Adds vitest as a devDependency only + a test script. No new runtime/production dependency, no postinstall/lifecycle scripts added.
AppNameField.tsx (new) Small, pure presentational component using existing EDS Input/Progress.Circular. No eval, no dynamic imports, no network/IO calls.
AppNameField.test.tsx (new) Unit test only, no I/O.
EditBookmarkModal.tsx Adds a isAppNameLoading boolean derived from existing observable state (appName, appNameError). No new external calls, no altered auth/data-fetch logic beyond the loading flag.
tsconfig.json Adds src/__tests__/**/* to exclude — standard for keeping tests out of the build output.
vitest.config.ts (new) Standard Vitest project config, scoped to src/__tests__. No custom reporters/plugins that could exfiltrate data.
pnpm-lock.yaml Only new resolved entry is vitest@4.1.4 (already a workspace-root devDependency elsewhere in the monorepo) with its existing peer resolution graph. No new/unknown top-level packages introduced.

Checks performed

  • ✅ Full diff manually reviewed line-by-line for exfiltration/obfuscation/eval/dynamic require/lifecycle-script patterns — none found.
  • ✅ No new runtime dependencies; only a well-known, already-used-in-repo devDependency (vitest) added, matching the version already pinned at the workspace root.
  • ✅ No changes to CI/workflow files, .npmrc, package.json scripts.* beyond a plain vitest --run test script, or any secrets/config files.
  • pnpm install after rebase pulled no unexpected packages and required no new "ignored build scripts" approval.
  • ✅ Rebased onto latest main, reran full validation:
    • npx vitest run for the package: 2/2 passed
    • tsc -b --force for the package: clean
    • pnpm exec biome lint packages/react/components/bookmark: clean
    • pnpm exec fusion-lint lint packages/react/components/bookmark: clean

Verdict

No security concerns identified. The change is a small, self-contained UI/accessibility fix with no new attack surface. Safe to proceed with normal review.


Update

Rebased this branch onto latest main (pushed to agent/fix-edit-bookmark-loading-skeleton, new head 5bf4aea44) and refreshed pnpm-lock.yaml to resolve peer-dependency drift from packages that moved on main since this PR was opened (@types/node, msw, jsdom, tsx). All validation above was re-run against the rebased branch.

@odinr
odinr force-pushed the agent/fix-edit-bookmark-loading-skeleton branch from 5bf4aea to 82507d1 Compare July 30, 2026 07:11
@odinr
odinr marked this pull request as ready for review July 30, 2026 07:12
@odinr
odinr requested a review from a team as a code owner July 30, 2026 07:12
@odinr
odinr requested a review from Copilot July 30, 2026 07:13

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 improves the Edit Bookmark modal UX in @equinor/fusion-framework-react-components-bookmark by making app-manifest resolution explicit: the app name field now shows a loading indicator while the manifest request is pending, and swaps to the resolved display name once available.

Changes:

  • Add AppNameField to render a read-only app name input with aria-busy and an EDS circular progress indicator during pending state.
  • Update EditBookmarkModal to compute a precise isAppNameLoading condition based on bookmark/provider presence, unresolved value, and no request error.
  • Introduce package-level Vitest setup (config + script) and a unit test for the loading/resolved rendering contract; include README + changeset updates.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pnpm-lock.yaml Locks the added Vitest dev dependency for the workspace.
packages/react/components/bookmark/vitest.config.ts Adds a Vitest project config for the bookmark components package.
packages/react/components/bookmark/tsconfig.json Excludes test files from the build TypeScript project.
packages/react/components/bookmark/src/components/edit-bookmark/EditBookmarkModal.tsx Computes manifest-loading state and swaps the raw Input for AppNameField.
packages/react/components/bookmark/src/components/edit-bookmark/AppNameField.tsx New component that renders the app name field with loading adornment + accessibility attributes.
packages/react/components/bookmark/src/tests/AppNameField.test.tsx Adds tests validating loading indicator + resolved rendering behavior.
packages/react/components/bookmark/README.md Documents loading feedback behavior in the edit modal feature list.
packages/react/components/bookmark/package.json Adds vitest devDependency and a test script for the package.
.changeset/react-components-bookmark_app-loading.md Patch changeset describing the user-facing loading feedback improvement.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

@odinr
odinr merged commit efad984 into equinor:main Jul 30, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Something isn't working 🚧 chore maintaines work, (update deps, workflos ...) 📚 documentation Improvements or additions to documentation 👾 React

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show loading skeleton for app name field in EditBookmark

3 participants