Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 1086930 - Add status region to sync calendar sync button #26386

Merged
merged 1 commit into from Jan 19, 2015
Merged

Bug 1086930 - Add status region to sync calendar sync button #26386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/calendar/elements/settings.html
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@ <h1></h1>
<button class="sync update toolbar-item">
<span class="icon"
data-l10n-id="drawer-sync-button">Sync</span>
<span class="sync-progress" role="progressbar"></span>
</button>
</div>
</div>
Expand Down
18 changes: 18 additions & 0 deletions apps/calendar/js/views/settings.js
View file
Open in desktop
Expand Up @@ -66,6 +66,7 @@ Settings.prototype = {

advancedSettingsButton: '#settings .settings',
syncButton: '#settings .sync',
syncProgress: '#settings .sync-progress',

// A view that settings overlays. Still needs to be active/visible but
// hidden from the screen reader.
Expand Down Expand Up @@ -108,17 +109,34 @@ Settings.prototype = {
return this._findElement('syncButton');
},

get syncProgress() {
return this._findElement('syncProgress');
},

get timeViews() {
return this._findElement('timeViews');
},

_syncStartStatus: function () {
this.syncProgress.setAttribute('data-l10n-id', 'sync-progress-syncing');
this.syncProgress.classList.add('syncing');
},

_syncCompleteStatus: function () {
this.syncProgress.setAttribute('data-l10n-id', 'sync-progress-complete');
this.syncProgress.classList.remove('syncing');
},

_observeUI: function() {
this.advancedSettingsButton.addEventListener('click', function(e) {
e.stopPropagation();
app.router.show('/advanced-settings/');
});

this.syncButton.addEventListener('click', this._onSyncClick.bind(this));
this.app.syncController.on('syncStart', this._syncStartStatus.bind(this));
this.app.syncController.on('syncComplete',
this._syncCompleteStatus.bind(this));

this.calendars.addEventListener(
'change', this._onCalendarDisplayToggle.bind(this)
Expand Down
6 changes: 6 additions & 0 deletions apps/calendar/locales/calendar.en-US.properties
View file
Open in desktop
Expand Up @@ -79,6 +79,12 @@ remove-account-dialog-details=Removing this account will also remove your local
# for accessibility purposes, not visible to the eye.
drawer-sync-button=Sync

# sync progress
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment like this: https://github.com/jobara/gaia/blob/1086930/apps/calendar/locales/calendar.en-US.properties#L254-L255 here

# The following strings are spoken by screen readers and not shown on the
# screen.
sync-progress-syncing.ariaValueText=Syncing
sync-progress-complete.ariaValueText=Syncing complete

# sync
sync-frequency=Sync Calendar
sync-frequency-15min=15 min
Expand Down
8 changes: 8 additions & 0 deletions apps/calendar/style/settings.css
View file
Open in desktop
Expand Up @@ -209,6 +209,14 @@ not actually shown since the top header should be transparent.
to { transform: rotate(360deg); }
}

.sync-progress {
visibility: hidden;
}

.sync-progress.syncing {
visibility: visible;
}

#settings [role="toolbar"].noaccount .sync {
display: none;
}
Expand Down
32 changes: 24 additions & 8 deletions apps/calendar/test/unit/views/settings_test.js
View file
Open in desktop
Expand Up @@ -84,6 +84,7 @@ suiteGroup('views/settings', function() {
' <button class="sync update toolbar-item">',
' <span class="icon"',
' data-l10n-id="drawer-sync-button">Sync</span>',
' <span class="sync-progress" role="progressbar"></span>',
' </button>',
' </div>',
' </div>',
Expand Down Expand Up @@ -170,6 +171,10 @@ suiteGroup('views/settings', function() {
assert.ok(subject.syncProgressTarget);
});

test('#syncProgress', function() {
assert.ok(subject.syncProgress);
});

suite('#_observeAccountStore', function() {
var accounts = testSupport.calendar.dbFixtures(
'account',
Expand Down Expand Up @@ -336,16 +341,27 @@ suiteGroup('views/settings', function() {
});
});

test('sync', function() {
var controller = app.syncController;
var calledWith;
suite('syncProgress', function() {

controller.all = function() {
calledWith = arguments;
};
test('syncStart', function(done) {
app.syncController.on('syncStart', function () {
assert.ok(subject.syncProgress.classList.contains('syncing'));
assert.equal(subject.syncProgress.getAttribute('data-l10n-id'),
'sync-progress-syncing');
Copy link
Contributor

Choose a reason for hiding this comment

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

these assertions are not working the way you expect.. controller.on('syncStart', fn) will be triggered asynchronously, so even if the assertions fails they should only fail after the test is already complete..

the proper way to test this is to create 2 async tests like:

suite('syncProgress', function() {
  var syncController = app.syncController;

  test('syncStart', function(done) {
    syncController.on('syncStart', function () {
      assert.ok(subject.syncProgress.classList.contains('syncing'));
      assert.equal(subject.syncProgress.getAttribute('data-l10n-id'),
      'sync-progress-syncing');
      done();
    });
    syncController.emit('syncStart');
  });

  test('syncComplete', function(done) {
    syncController.on('syncComplete', function () {
      assert.equal(subject.syncProgress.getAttribute('data-l10n-id'),
      'sync-progress-complete');
      assert.notOk(subject.syncProgress.classList.contains('syncing'));
      done();
    });
    syncController.emit('syncComplete');
  });
});

or if you are really want to test the click event, you should use the waitFor pattern

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@millermedeiros thanks for the explanation. I made the changes you recommended as it's probably enough in this case to test the actions of the events. The only change I made was to call app.syncController directly in the test blocks as app does not seem to be in scope for the suite.

done();
});
app.syncController.emit('syncStart');
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Also check before triggering the initial state

triggerEvent(subject.syncButton, 'click');
assert.ok(calledWith);
test('syncComplete', function(done) {
app.syncController.on('syncComplete', function () {
assert.equal(subject.syncProgress.getAttribute('data-l10n-id'),
'sync-progress-complete');
assert.notOk(subject.syncProgress.classList.contains('syncing'));
done();
});
app.syncController.emit('syncComplete');
});
});

suite('#_onCalendarDisplayToggle', function() {
Expand Down