Fix#20230: Disable the discard button until Autosave is in progress. (o…

…ppia#20231) * Disable discard button when autosave is in progress for mobile and desktop * Disable discard button when autosave is in progress for mobile and desktop * Make the selector more specific * correct the strings * remove comment
AFZL210 · May 8, 2024 · c57292a · c57292a
1 parent 5ebbb3d
commit c57292a
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 4 deletions.
diff --git a/...emplates/pages/exploration-editor-page/editor-navigation/editor-navigation.component.html b/...emplates/pages/exploration-editor-page/editor-navigation/editor-navigation.component.html
@@ -119,7 +119,8 @@
         </div>
 
         <ul class="dropdown-menu" aria-labelledby="discardButtonPopup">
-          <div class="e2e-test-mobile-exploration-discard-tab" (click)="discardChanges()">
+<!-- The 'disabled' class is added when autosave is in progress. This disables user interactions with the discard button, preventing changes from being discarded before they are saved. -->
+          <div class="e2e-test-mobile-exploration-discard-tab" (click)="discardChanges()" [class.disabled]="autosaveIsInProgress">
             <i class="fa fa-pen oppia-mobile-exploration-editor-tabs-icon"></i>
             Discard
           </div>
@@ -155,6 +156,11 @@
   .nav > li > a.oppia-editor-navbar-tab-anchor {
     padding: 15px 11px;
   }
+  /* Ensures targeted styling only affects 'Discard Draft' tab. */
+  div.e2e-test-mobile-exploration-discard-tab.disabled {
+    opacity: 0.6;
+    pointer-events: none;
+  }
   .oppia-exploration-open-threads-count {
     background-color: #4078c0;
     border: 1px solid;

diff --git a/...lates/pages/exploration-editor-page/editor-navigation/editor-navigation.component.spec.ts b/...lates/pages/exploration-editor-page/editor-navigation/editor-navigation.component.spec.ts
@@ -315,6 +315,28 @@ describe('Editor Navigation Component', () => {
       expect(component.isEditableOutsideTutorialMode()).toEqual(true);
     });
 
+    it('should update autosaveIsInProgress when autosaveInProgressEventEmitter emits true', () => {
+      spyOn(
+        changeListService.autosaveInProgressEventEmitter,
+        'subscribe'
+      ).and.callFake((callback: (autosaveInProgress: boolean) => void) => {
+        callback(true);
+      });
+      component.ngOnInit();
+      expect(component.autosaveIsInProgress).toBe(true);
+    });
+
+    it('should update autosaveIsInProgress when autosaveInProgressEventEmitter emits false', () => {
+      spyOn(
+        changeListService.autosaveInProgressEventEmitter,
+        'subscribe'
+      ).and.callFake((callback: (autosaveInProgress: boolean) => void) => {
+        callback(false);
+      });
+      component.ngOnInit();
+      expect(component.autosaveIsInProgress).toBe(false);
+    });
+
     it('should call exploration save service to discard changes', () => {
       let explorationSpy = spyOn(explorationSaveService, 'discardChanges');
       component.discardChanges();

diff --git a/.../templates/pages/exploration-editor-page/editor-navigation/editor-navigation.component.ts b/.../templates/pages/exploration-editor-page/editor-navigation/editor-navigation.component.ts
@@ -44,6 +44,7 @@ import {UserExplorationPermissionsService} from '../services/user-exploration-pe
 })
 export class EditorNavigationComponent implements OnInit, OnDestroy {
   directiveSubscriptions = new Subscription();
+  autosaveIsInProgress: boolean = false;
   screenIsLarge: boolean = false;
   isPublishButtonEnabled: boolean = false;
   postTutorialHelpPopoverIsShown: boolean = false;
@@ -280,6 +281,14 @@ export class EditorNavigationComponent implements OnInit, OnDestroy {
       )
     );
 
+    this.directiveSubscriptions.add(
+      this.changeListService.autosaveInProgressEventEmitter.subscribe(
+        (autosaveInProgress: boolean) => {
+          this.autosaveIsInProgress = autosaveInProgress;
+        }
+      )
+    );
+
     this.directiveSubscriptions.add(
       this.internetConnectivityService.onInternetStateChange.subscribe(
         internetAccessible => {

diff --git a/.../exploration-save-and-publish-buttons/exploration-save-and-publish-buttons.component.html b/.../exploration-save-and-publish-buttons/exploration-save-and-publish-buttons.component.html
@@ -67,13 +67,12 @@ <h3 class="e2e-test-joyride-title" tabindex="0">Save</h3>
           <loading-dots [hidden]="!loadingDotsAreShown"></loading-dots>
         </span>
       </button>
-
       <button [matMenuTriggerFor]="discardButton" type="button" class="btn btn-light oppia-discard-button e2e-test-save-discard-toggle dropdown-toggle"
-              [disabled]="!connectedToInternet || !getChangeListLength()" aria-label="Discard Draft">
+              [disabled]="!connectedToInternet || !getChangeListLength() || autosaveIsInProgress" aria-label="Discard Draft">
       </button>
 
       <mat-menu #discardButton="matMenu" [ngStyle]="{ width: getChangeListLength() && !isPrivate() ? '150px' : '120px' }">
-        <button mat-menu-item title="Discard all pending changes"><a (click)="discardChanges()" [ngClass]="{'oppia-disabled-link': !connectedToInternet || !getChangeListLength()}" class="dropdown-item nav-item e2e-test-discard-changes">Discard Draft</a></button>
+        <button mat-menu-item title="Discard all pending changes"><a (click)="discardChanges()" [ngClass]="{'oppia-disabled-link': !connectedToInternet || !getChangeListLength() || autosaveIsInProgress}" class="dropdown-item nav-item e2e-test-discard-changes">Discard Draft</a></button>
       </mat-menu>
     </div>
   </li>

diff --git a/...ploration-save-and-publish-buttons/exploration-save-and-publish-buttons.component.spec.ts b/...ploration-save-and-publish-buttons/exploration-save-and-publish-buttons.component.spec.ts
@@ -264,6 +264,28 @@ describe('Exploration save and publish buttons component', () => {
     }
   );
 
+  it('should update autosaveIsInProgress when autosaveInProgressEventEmitter emits true', () => {
+    spyOn(
+      changeListService.autosaveInProgressEventEmitter,
+      'subscribe'
+    ).and.callFake((callback: (autosaveInProgress: boolean) => void) => {
+      callback(true);
+    });
+    component.ngOnInit();
+    expect(component.autosaveIsInProgress).toBe(true);
+  });
+
+  it('should update autosaveIsInProgress when autosaveInProgressEventEmitter emits false', () => {
+    spyOn(
+      changeListService.autosaveInProgressEventEmitter,
+      'subscribe'
+    ).and.callFake((callback: (autosaveInProgress: boolean) => void) => {
+      callback(false);
+    });
+    component.ngOnInit();
+    expect(component.autosaveIsInProgress).toBe(false);
+  });
+
   it(
     'should publish exploration changes when it has no warnings and it is' +
       ' public',

diff --git a/...ge/exploration-save-and-publish-buttons/exploration-save-and-publish-buttons.component.ts b/...ge/exploration-save-and-publish-buttons/exploration-save-and-publish-buttons.component.ts
@@ -40,6 +40,7 @@ export class ExplorationSaveAndPublishButtonsComponent
   directiveSubscriptions = new Subscription();
 
   isModalDisplayed: boolean = false;
+  autosaveIsInProgress: boolean;
   saveIsInProcess: boolean;
   publishIsInProcess: boolean;
   loadingDotsAreShown: boolean;
@@ -216,6 +217,14 @@ export class ExplorationSaveAndPublishButtonsComponent
       )
     );
 
+    this.directiveSubscriptions.add(
+      this.changeListService.autosaveInProgressEventEmitter.subscribe(
+        (autosaveInProgress: boolean) => {
+          this.autosaveIsInProgress = autosaveInProgress;
+        }
+      )
+    );
+
     this.directiveSubscriptions.add(
       this.internetConnectivityService.onInternetStateChange.subscribe(
         internetAccessible => {