Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace selection tabs with view switcher (feature flagged) #465

Merged
merged 2 commits into from
Jun 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/sidebar/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ module.exports = angular.module('h', [
.component('threadList', require('./components/thread-list'))
.component('timestamp', require('./components/timestamp'))
.component('topBar', require('./components/top-bar'))
.component('viewSwitcher', require('./components/view-switcher'))

.directive('formInput', require('./directive/form-input'))
.directive('formValidate', require('./directive/form-validate'))
Expand Down
1 change: 1 addition & 0 deletions src/sidebar/components/sidebar-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ function SidebarContentController(
totalNotes: counts.notes,
totalAnnotations: counts.annotations,
totalOrphans: counts.orphans,
viewSwitcherEnabled: features.flagEnabled('view-switcher'),
waitingToAnchorAnnotations: counts.anchoring > 0,
});
});
Expand Down
63 changes: 63 additions & 0 deletions src/sidebar/components/test/view-switcher-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'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 = {};
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'));
});
});
});
42 changes: 42 additions & 0 deletions src/sidebar/components/view-switcher.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'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.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'),
};
11 changes: 10 additions & 1 deletion src/sidebar/templates/sidebar-content.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
<selection-tabs
ng-if="!vm.search.query() && vm.selectedAnnotationCount() === 0"
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">
</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
ng-show="!vm.isLoading()"
Expand Down
57 changes: 57 additions & 0 deletions src/sidebar/templates/view-switcher.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<div class="view-switcher">
<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>
1 change: 1 addition & 0 deletions src/styles/app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ $base-line-height: 20px;
@import './thread-list';
@import './tooltip';
@import './top-bar';
@import './view-switcher';

// Top-level styles
// ----------------
Expand Down
61 changes: 61 additions & 0 deletions src/styles/view-switcher.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
.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.
Copy link
Member

Choose a reason for hiding this comment

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

These comments are very helpful. If we can find a way to do the alignment "in code" via shared variables etc. that's even better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. I'm not sure exactly what would be required to achieve that though - for example I'm not sure whether the existing elements are aligned to a grid (and therefore the grid size could be a variable that we could use multiples of). If it's not aligned to a grid and some of the gaps are seemingly arbitrary then we might end up with a lot of difficult to name variables. So needs investigation.

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;

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;
Copy link
Member

Choose a reason for hiding this comment

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

Do we have an existing color from our palette that we can use here?

Copy link
Contributor Author

@seanh seanh Jun 28, 2017

Choose a reason for hiding this comment

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

To be honest, I don't think so. $grey-1 and $grey-2 are too light in my opinion (not different enough from the unselected colour) and then $grey-3 is way too dark. There's no example that I'm aware of of an old-style Hypothesis button or tab that has a depressed state like this one. I tried mine-shaft, dove-gray etc as well but didn't think any of them worked. (I can't remember where I picked this particular shade from originally, can't find it anywhere else in the app now!)

}

.view-switcher__empty-message {
position: relative;
top: 10px;
}