Remove the view switcher component

Remove the (feature-flagged) view switcher component.

We missed the chance to strike while the iron was hot on this one -
neither Dawa or I have time anymore to work on further design
iterations. I don't want to have dead code living behind a feature flag
so I'm removing it. The code will still be in git if we ever want to
revive it. Perhaps these tabs can be re-designed as part of a larger
re-design of the sidebar in the future.

I'll leave the GitHub issue about the usability issues with the existing
selection tabs open: https://github.com/hypothesis/product-backlog/issues/327

For the record the main remaining issues with this were:

1. An undesirable visual popping happens when the view switcher loads on
pages that have a lot of annotations. This also happens with the
existing selection tabs but is arguably more visible with the view
switcher.

2. When there are only two tabs (no orphans) the visual design of the
tabs doesn't make it as immediately obvious which is the currently
selected tab. The other tab may be mistaken for the selected one.

The TODO list was here:

https://github.com/hypothesis/product-backlog/issues/327#issuecomment-311073581

The pull requests for this view switcher were:

https://github.com/hypothesis/client/pull/429
https://github.com/hypothesis/client/pull/481
https://github.com/hypothesis/client/pull/482
https://github.com/hypothesis/product-backlog/issues/327
parent 19505853
...@@ -177,7 +177,6 @@ module.exports = angular.module('h', [ ...@@ -177,7 +177,6 @@ module.exports = angular.module('h', [
.component('threadList', require('./components/thread-list')) .component('threadList', require('./components/thread-list'))
.component('timestamp', require('./components/timestamp')) .component('timestamp', require('./components/timestamp'))
.component('topBar', require('./components/top-bar')) .component('topBar', require('./components/top-bar'))
.component('viewSwitcher', require('./components/view-switcher'))
.directive('formInput', require('./directive/form-input')) .directive('formInput', require('./directive/form-input'))
.directive('formValidate', require('./directive/form-validate')) .directive('formValidate', require('./directive/form-validate'))
......
...@@ -55,7 +55,6 @@ function SidebarContentController( ...@@ -55,7 +55,6 @@ function SidebarContentController(
totalNotes: counts.notes, totalNotes: counts.notes,
totalAnnotations: counts.annotations, totalAnnotations: counts.annotations,
totalOrphans: counts.orphans, totalOrphans: counts.orphans,
viewSwitcherEnabled: features.flagEnabled('view-switcher'),
waitingToAnchorAnnotations: counts.anchoring > 0, waitingToAnchorAnnotations: counts.anchoring > 0,
}); });
}); });
......
'use strict';
var angular = require('angular');
var util = require('../../directive/test/util');
describe('viewSwitcher', function () {
before(function () {
angular.module('app', [])
.component('viewSwitcher', require('../view-switcher'));
});
beforeEach(function () {
var fakeAnnotationUI = {
getState: sinon.stub().returns({
frames: [
{
// The view switcher only shows after the first batch of
// annotations have been fetched.
isAnnotationFetchComplete: true,
},
],
}),
};
var fakeFeatures = {
flagEnabled: sinon.stub().returns(true),
};
angular.mock.module('app', {
annotationUI: fakeAnnotationUI,
features: fakeFeatures,
});
});
context('displays tabs, counts and selected tab', function () {
it('should display the tabs and counts of annotations and notes', function () {
var elem = util.createDirective(document, 'viewSwitcher', {
selectedTab: 'annotation',
totalAnnotations: '123',
totalNotes: '456',
});
var tabs = elem[0].querySelectorAll('button');
assert.include(tabs[0].textContent, 'Annotations');
assert.include(tabs[1].textContent, 'Notes');
assert.include(tabs[0].textContent, '123');
assert.include(tabs[1].textContent, '456');
});
it('should display annotations tab as selected', function () {
var elem = util.createDirective(document, 'viewSwitcher', {
selectedTab: 'annotation',
totalAnnotations: '123',
totalNotes: '456',
});
var tabs = elem[0].querySelectorAll('button');
assert.isTrue(tabs[0].classList.contains('is-selected'));
});
it('should display notes tab as selected', function () {
var elem = util.createDirective(document, 'viewSwitcher', {
selectedTab: 'note',
totalAnnotations: '123',
totalNotes: '456',
});
var tabs = elem[0].querySelectorAll('button');
assert.isTrue(tabs[1].classList.contains('is-selected'));
});
});
});
'use strict';
var uiConstants = require('../ui-constants');
module.exports = {
controllerAs: 'vm',
//@ngInject
controller: function ($element, annotationUI, features) {
this.TAB_ANNOTATIONS = uiConstants.TAB_ANNOTATIONS;
this.TAB_NOTES = uiConstants.TAB_NOTES;
this.TAB_ORPHANS = uiConstants.TAB_ORPHANS;
this.selectTab = function (type) {
annotationUI.clearSelectedAnnotations();
annotationUI.selectTab(type);
};
this.orphansTabFlagEnabled = function () {
return features.flagEnabled('orphans_tab');
};
this.showViewSwitcher = function() {
var frame = annotationUI.getState().frames[0];
if (frame && frame.isAnnotationFetchComplete) {
return true;
}
return false;
};
this.showAnnotationsUnavailableMessage = function () {
return this.selectedTab === this.TAB_ANNOTATIONS &&
this.totalAnnotations === 0 &&
!this.isWaitingToAnchorAnnotations;
};
this.showNotesUnavailableMessage = function () {
return this.selectedTab === this.TAB_NOTES &&
this.totalNotes === 0;
};
},
bindings: {
isLoading: '<',
isWaitingToAnchorAnnotations: '<',
selectedTab: '<',
totalAnnotations: '<',
totalNotes: '<',
totalOrphans: '<',
},
template: require('../templates/view-switcher.html'),
};
<selection-tabs <selection-tabs
ng-if="!vm.viewSwitcherEnabled && !vm.search.query() && vm.selectedAnnotationCount() === 0" ng-if="!vm.search.query() && vm.selectedAnnotationCount() === 0"
is-waiting-to-anchor-annotations="vm.waitingToAnchorAnnotations" is-waiting-to-anchor-annotations="vm.waitingToAnchorAnnotations"
is-loading="vm.isLoading" is-loading="vm.isLoading"
selected-tab="vm.selectedTab" selected-tab="vm.selectedTab"
...@@ -7,15 +7,6 @@ ...@@ -7,15 +7,6 @@
total-notes="vm.totalNotes" total-notes="vm.totalNotes"
total-orphans="vm.totalOrphans"> total-orphans="vm.totalOrphans">
</selection-tabs> </selection-tabs>
<view-switcher
ng-if="vm.viewSwitcherEnabled && !vm.search.query() && vm.selectedAnnotationCount() === 0"
is-waiting-to-anchor-annotations="vm.waitingToAnchorAnnotations"
is-loading="vm.isLoading"
selected-tab="vm.selectedTab"
total-annotations="vm.totalAnnotations"
total-notes="vm.totalNotes"
total-orphans="vm.totalOrphans">
</view-switcher>
<search-status-bar <search-status-bar
ng-show="!vm.isLoading()" ng-show="!vm.isLoading()"
......
<div class="view-switcher" ng-if="vm.showViewSwitcher()">
<button class="view-switcher__tab"
ng-class="{'is-selected': vm.selectedTab === vm.TAB_ANNOTATIONS}"
h-on-touch="vm.selectTab(vm.TAB_ANNOTATIONS)">
<span class="view-switcher__tab-label">
Annotations
</span>
<span class="view-switcher__tab-count"
ng-if="vm.totalAnnotations > 0 && !vm.isWaitingToAnchorAnnotations">
{{ vm.totalAnnotations }}
</span>
</button>
<button class="view-switcher__tab"
ng-class="{'is-selected': vm.selectedTab === vm.TAB_NOTES}"
h-on-touch="vm.selectTab(vm.TAB_NOTES)">
<span class="view-switcher__tab-label">
Page Notes
</span>
<span class="view-switcher__tab-count"
ng-if="vm.totalNotes > 0 && !vm.isWaitingToAnchorAnnotations">
{{ vm.totalNotes }}
</span>
</button>
<button class="view-switcher__tab view-switcher__tab--orphan"
ng-class="{'is-selected': vm.selectedTab === vm.TAB_ORPHANS}"
h-on-touch="vm.selectTab(vm.TAB_ORPHANS)"
ng-if="vm.totalOrphans > 0 && !vm.isWaitingToAnchorAnnotations">
<span class="view-switcher__tab-label">
Orphans
</span>
<span class="view-switcher__tab-count">
{{ vm.totalOrphans }}
</span>
</button>
</div>
<div ng-if="!vm.isLoading()" class="view-switcher__empty-message">
<div ng-if="vm.showNotesUnavailableMessage()" class="annotation-unavailable-message">
<p class="annotation-unavailable-message__label">
There are no page notes in this group.
<br />
Create one by clicking the
<i class="help-icon h-icon-note"></i>
button.
</p>
</div>
<div ng-if="vm.showAnnotationsUnavailableMessage()" class="annotation-unavailable-message">
<p class="annotation-unavailable-message__label">
There are no annotations in this group.
<br />
Create one by selecting some text and clicking the
<i class="help-icon h-icon-annotate"></i> button.
</p>
</div>
</div>
...@@ -31,7 +31,6 @@ $base-line-height: 20px; ...@@ -31,7 +31,6 @@ $base-line-height: 20px;
@import './thread-list'; @import './thread-list';
@import './tooltip'; @import './tooltip';
@import './top-bar'; @import './top-bar';
@import './view-switcher';
// Top-level styles // Top-level styles
// ---------------- // ----------------
......
.view-switcher {
display: flex;
justify-content: center;
// These are the exact margins required to vertically align the top of the
// view switcher with the top of the Hide Highlights button to its left,
// and the top of the first annotation card below the view switcher with the
// top of the New Page Note button to its left.
margin-top: 1px;
margin-bottom: 6px;
}
.view-switcher__tab {
@include smallshadow
height: 30px; // Same height as used for Hide Highlights and New Page Note
// buttons to left of view switcher.
padding-left: 12px;
padding-right: 12px;
background: $white;
transition: background-color .1s linear;
// This fixes the tabs flashing gray for a moment each time you tap on one
// in iOS Safari.
-webkit-tap-highlight-color: rgba(0,0,0,0);
border: 1px solid $gray-lighter;
cursor: pointer;
user-select: none;
}
.view-switcher__tab {
border-right-width: 0;
}
.view-switcher__tab:last-child {
border-right-width: 1px;
}
.view-switcher__tab:first-child {
border-top-left-radius: 4px;
border-bottom-left-radius: 4px;
}
.view-switcher__tab:last-child {
border-top-right-radius: 4px;
border-bottom-right-radius: 4px;
}
.view-switcher__tab:focus {
outline: 0;
}
.view-switcher__tab::-moz-focus-inner {
border: 0;
}
.view-switcher__tab:hover,
.view-switcher__tab.is-selected {
background-color: #e6e6e6;
}
.view-switcher__empty-message {
position: relative;
top: 10px;
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment