Skip to content

fix: disable 'Update App' button when no changes are made#7975

Open
Syed-Moiz-Ali wants to merge 2 commits into
BasedHardware:mainfrom
Syed-Moiz-Ali:fix/3559-only
Open

fix: disable 'Update App' button when no changes are made#7975
Syed-Moiz-Ali wants to merge 2 commits into
BasedHardware:mainfrom
Syed-Moiz-Ali:fix/3559-only

Conversation

@Syed-Moiz-Ali

Copy link
Copy Markdown

Fixes #3559

Problem

The 'Update App' button appears enabled even when the user has not changed any fields, allowing no-op updates.

Changes

  • Store original app reference (_originalApp) when prepareUpdate is called
  • Added hasChanges flag to AddAppProvider that compares current form state against original app data
  • Button is now disabled when !isValid || !hasChanges
  • Visual state (color) reflects the disabled state (grey when disabled)

@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a hasChanges guard to the Update App button so it stays disabled until the user actually modifies the form. It stores the original App in _originalApp during prepareUpdate, wires hasDataChanged into checkValidity, and expands the diff check to cover additional fields (payment, thumbnails, auth integration URLs).

  • add_app_provider.dart: Introduces _originalApp / hasChanges state, extends hasDataChanged with new field comparisons, and drives the flag from checkValidity.
  • update_app.dart: Button onTap guard and color expression now gate on both isValid && hasChanges.

Confidence Score: 3/5

Two defects in hasDataChanged can cause the button to misbehave in opposite directions — incorrectly enabled by null vs empty-string comparisons, or incorrectly disabled by count-only capability checks.

The nullable URL field comparisons will silently report a phantom change on any app where those fields are null, defeating the guard. The capability count check allows a swap of capabilities to go undetected, blocking a legitimate update.

app/lib/pages/apps/providers/add_app_provider.dart — hasDataChanged needs null-safe URL comparisons and identity-based capability comparison.

Important Files Changed

Filename Overview
app/lib/pages/apps/providers/add_app_provider.dart Adds _originalApp reference and hasChanges flag; hasDataChanged is extended but has two logic bugs: null vs empty-string comparisons on nullable external integration fields produce false positives, and capability comparison is by count only (not by identity), so a capability swap is invisible to the check.
app/lib/pages/apps/update_app.dart Button onTap guard and color expression updated to include hasChanges; logic is correct but the inner widget tree has a minor indentation inconsistency introduced by the diff.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User opens UpdateAppPage] --> B[prepareUpdate called\nisValid=false, hasChanges=false\n_originalApp stored]
    B --> C[Button disabled\ngrey colour]
    C --> D{User edits a field}
    D -->|triggers checkValidity| E[isFormValid evaluated\nhasDataChanged vs _originalApp evaluated]
    E --> F{isValid AND hasChanges?}
    F -->|Yes| G[Button enabled\nwhite colour]
    F -->|No| C
    G --> H{User taps Update}
    H --> I[validateForm]
    I -->|valid| J[Confirmation dialog]
    J -->|confirm| K[updateApp called]
    K --> L[clear + checkValidity\nNavigator.pop on success]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User opens UpdateAppPage] --> B[prepareUpdate called\nisValid=false, hasChanges=false\n_originalApp stored]
    B --> C[Button disabled\ngrey colour]
    C --> D{User edits a field}
    D -->|triggers checkValidity| E[isFormValid evaluated\nhasDataChanged vs _originalApp evaluated]
    E --> F{isValid AND hasChanges?}
    F -->|Yes| G[Button enabled\nwhite colour]
    F -->|No| C
    G --> H{User taps Update}
    H --> I[validateForm]
    I -->|valid| J[Confirmation dialog]
    J -->|confirm| K[updateApp called]
    K --> L[clear + checkValidity\nNavigator.pop on success]
Loading

Comments Outside Diff (1)

  1. app/lib/pages/apps/update_app.dart, line 491-503 (link)

    P2 Inconsistent indentation after refactor

    The inner child: Container(…) and its child: Text(…) are now indented two levels past the GestureDetector, creating an irregular nesting that doesn't match the surrounding widget tree style. The Text widget sits at the same column as Container's body rather than being indented within it. This is a formatting-only issue but makes the widget hierarchy harder to follow.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "fix: disable Update App button when no c..." | Re-trigger Greptile

Comment thread app/lib/pages/apps/providers/add_app_provider.dart Outdated
Comment thread app/lib/pages/apps/providers/add_app_provider.dart Outdated
@Syed-Moiz-Ali

Copy link
Copy Markdown
Author

Fixed both issues:

  1. Null vs empty-string — Added ?? ''\ to all nullable field comparisons (webhookUrl, setupCompletedUrl, instructionsFilePath, appHomeUrl, chatToolsManifestUrl) so that \ != null\ no longer falsely triggers \hasChanges = true.

  2. Capability swap — Changed from comparing \selectedCapabilities.length != app.capabilities.length\ to comparing sorted capability ID lists via \listEquals(...).

@Syed-Moiz-Ali Syed-Moiz-Ali left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

retest

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mobile UX change (disable Update App button when no changes, #3559) — approve only per policy.

@ThomsenDrake

Copy link
Copy Markdown
Collaborator

ClawSweeper local pilot review

Recommendation: keep open, but do not merge as-is.

What it found:

  • The dirty-state guard targets a real Update App UX bug: current main gates the button on validity, not whether anything changed.
  • The implementation can still leave legitimate edits blocked because not every editable update-page field appears to recompute dirty state.
  • Example risk: editing only the source-code URL or pricing controls can leave hasChanges false and keep the Update App button disabled.
  • Real app proof is missing.

Suggested next step: recompute dirty state from every editable update-page field, then add redacted app proof showing the button disabled on load, enabled after a valid edit, and disabled again after reverting.

Posted from a local report-only ClawSweeper pilot by request; no labels, closes, repairs, or merges were performed.

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.

Disable "Update App" button in Omi app store when no changes are made

4 participants