Unverified Commit 09e54144 authored by Sean Hammond's avatar Sean Hammond Committed by GitHub

Merge pull request #578 from hypothesis/remove-orphans-tab-flag

Remove orphans tab feature flag checks
parents 286dfbc5 093ef18a
...@@ -447,14 +447,7 @@ function AnnotationController( ...@@ -447,14 +447,7 @@ function AnnotationController(
return serviceUrl('search.tag', {tag: tag}); return serviceUrl('search.tag', {tag: tag});
}; };
// Note: We fetch the feature flag outside the `isOrphan` method to avoid a
// lookup on every $digest cycle
var indicateOrphans = features.flagEnabled('orphans_tab');
this.isOrphan = function() { this.isOrphan = function() {
if (!indicateOrphans) {
return false;
}
if (typeof self.annotation.$orphan === 'undefined') { if (typeof self.annotation.$orphan === 'undefined') {
return self.annotation.$anchorTimeout; return self.annotation.$anchorTimeout;
} }
......
...@@ -19,10 +19,6 @@ module.exports = { ...@@ -19,10 +19,6 @@ module.exports = {
annotationUI.selectTab(type); annotationUI.selectTab(type);
}; };
this.orphansTabFlagEnabled = function () {
return features.flagEnabled('orphans_tab');
};
this.showAnnotationsUnavailableMessage = function () { this.showAnnotationsUnavailableMessage = function () {
return this.selectedTab === this.TAB_ANNOTATIONS && return this.selectedTab === this.TAB_ANNOTATIONS &&
this.totalAnnotations === 0 && this.totalAnnotations === 0 &&
......
...@@ -48,8 +48,7 @@ function SidebarContentController( ...@@ -48,8 +48,7 @@ function SidebarContentController(
self.rootThread = thread(); self.rootThread = thread();
self.selectedTab = state.selectedTab; self.selectedTab = state.selectedTab;
var separateOrphans = tabs.shouldSeparateOrphans(state); var counts = tabs.counts(state.annotations);
var counts = tabs.counts(state.annotations, separateOrphans);
Object.assign(self, { Object.assign(self, {
totalNotes: counts.notes, totalNotes: counts.notes,
...@@ -227,8 +226,7 @@ function SidebarContentController( ...@@ -227,8 +226,7 @@ function SidebarContentController(
focusAnnotation(selectedAnnot); focusAnnotation(selectedAnnot);
scrollToAnnotation(selectedAnnot); scrollToAnnotation(selectedAnnot);
var separateOrphans = tabs.shouldSeparateOrphans(annotationUI.getState()); annotationUI.selectTab(tabs.tabForAnnotation(selectedAnnot));
annotationUI.selectTab(tabs.tabForAnnotation(selectedAnnot, separateOrphans));
}); });
// Re-fetch annotations when focused group, logged-in user or connected frames // Re-fetch annotations when focused group, logged-in user or connected frames
......
...@@ -66,9 +66,7 @@ function RootThread($rootScope, annotationUI, drafts, searchFilter, viewFilter) ...@@ -66,9 +66,7 @@ function RootThread($rootScope, annotationUI, drafts, searchFilter, viewFilter)
return false; return false;
} }
var separateOrphans = tabs.shouldSeparateOrphans(state); return tabs.shouldShowInTab(thread.annotation, state.selectedTab);
return tabs.shouldShowInTab(thread.annotation, state.selectedTab,
separateOrphans);
}; };
} }
......
...@@ -5,27 +5,15 @@ ...@@ -5,27 +5,15 @@
var countIf = require('./util/array-util').countIf; var countIf = require('./util/array-util').countIf;
var metadata = require('./annotation-metadata'); var metadata = require('./annotation-metadata');
var session = require('./reducers/session');
var uiConstants = require('./ui-constants'); var uiConstants = require('./ui-constants');
/**
* Return true if Annotations and Orphans should be displayed in separate tabs.
*
* @param {object} state - The current application state.
*/
function shouldSeparateOrphans(state) {
return session.isFeatureEnabled(state, 'orphans_tab');
}
/** /**
* Return the tab in which an annotation should be displayed. * Return the tab in which an annotation should be displayed.
* *
* @param {Annotation} ann * @param {Annotation} ann
* @param {boolean} separateOrphans - True if orphans should be displayed in a
* separate tab.
*/ */
function tabForAnnotation(ann, separateOrphans) { function tabForAnnotation(ann) {
if (metadata.isOrphan(ann) && separateOrphans) { if (metadata.isOrphan(ann)) {
return uiConstants.TAB_ORPHANS; return uiConstants.TAB_ORPHANS;
} else if (metadata.isPageNote(ann)) { } else if (metadata.isPageNote(ann)) {
return uiConstants.TAB_NOTES; return uiConstants.TAB_NOTES;
...@@ -39,26 +27,22 @@ function tabForAnnotation(ann, separateOrphans) { ...@@ -39,26 +27,22 @@ function tabForAnnotation(ann, separateOrphans) {
* *
* @param {Annotation} ann * @param {Annotation} ann
* @param {number} tab - The TAB_* value indicating the tab * @param {number} tab - The TAB_* value indicating the tab
* @param {boolean} separateOrphans - True if orphans should be separated into
* their own tab.
*/ */
function shouldShowInTab(ann, tab, separateOrphans) { function shouldShowInTab(ann, tab) {
if (metadata.isWaitingToAnchor(ann) && separateOrphans) { if (metadata.isWaitingToAnchor(ann)) {
// Until this annotation anchors or fails to anchor, we do not know which // Until this annotation anchors or fails to anchor, we do not know which
// tab it should be displayed in. // tab it should be displayed in.
return false; return false;
} }
return tabForAnnotation(ann, separateOrphans) === tab; return tabForAnnotation(ann) === tab;
} }
/** /**
* Return the counts for the headings of different tabs. * Return the counts for the headings of different tabs.
* *
* @param {Annotation[]} annotations - List of annotations to display * @param {Annotation[]} annotations - List of annotations to display
* @param {boolean} separateOrphans - True if orphans should be displayed in a
* separate tab.
*/ */
function counts(annotations, separateOrphans) { function counts(annotations) {
var counts = { var counts = {
notes: countIf(annotations, metadata.isPageNote), notes: countIf(annotations, metadata.isPageNote),
annotations: countIf(annotations, metadata.isAnnotation), annotations: countIf(annotations, metadata.isAnnotation),
...@@ -66,19 +50,11 @@ function counts(annotations, separateOrphans) { ...@@ -66,19 +50,11 @@ function counts(annotations, separateOrphans) {
anchoring: countIf(annotations, metadata.isWaitingToAnchor), anchoring: countIf(annotations, metadata.isWaitingToAnchor),
}; };
if (separateOrphans) { return counts;
return counts;
} else {
return Object.assign({}, counts, {
annotations: counts.annotations + counts.orphans,
orphans: 0,
});
}
} }
module.exports = { module.exports = {
counts: counts, counts: counts,
shouldSeparateOrphans: shouldSeparateOrphans,
shouldShowInTab: shouldShowInTab, shouldShowInTab: shouldShowInTab,
tabForAnnotation: tabForAnnotation, tabForAnnotation: tabForAnnotation,
}; };
...@@ -22,7 +22,7 @@ ...@@ -22,7 +22,7 @@
</span> </span>
</a> </a>
<a class="selection-tabs__type selection-tabs__type--orphan" <a class="selection-tabs__type selection-tabs__type--orphan"
ng-if="vm.orphansTabFlagEnabled() && vm.totalOrphans > 0" ng-if="vm.totalOrphans > 0"
href="#" href="#"
ng-class="{'is-selected': vm.selectedTab === vm.TAB_ORPHANS}" ng-class="{'is-selected': vm.selectedTab === vm.TAB_ORPHANS}"
h-on-touch="vm.selectTab(vm.TAB_ORPHANS)"> h-on-touch="vm.selectTab(vm.TAB_ORPHANS)">
......
...@@ -46,7 +46,7 @@ describe('rootThread', function () { ...@@ -46,7 +46,7 @@ describe('rootThread', function () {
isSidebar: true, isSidebar: true,
selectedAnnotationMap: null, selectedAnnotationMap: null,
session: { session: {
features: {'orphans_tab': true}, features: {},
}, },
sortKey: 'Location', sortKey: 'Location',
sortKeysAvailable: ['Location'], sortKeysAvailable: ['Location'],
......
...@@ -6,31 +6,11 @@ var tabs = require('../tabs'); ...@@ -6,31 +6,11 @@ var tabs = require('../tabs');
var unroll = require('../../shared/test/util').unroll; var unroll = require('../../shared/test/util').unroll;
describe('tabs', function () { describe('tabs', function () {
describe('shouldSeparateOrphans', function () {
it('returns true if the "orphans_tab" flag is enabled', function () {
var state = {
session: {
features: {'orphans_tab': true},
},
};
assert.isTrue(tabs.shouldSeparateOrphans(state));
});
it('returns false if the "orphans_tab" flag is not enabled', function () {
var state = {
session: {
features: {'orphans_tab': false},
},
};
assert.isFalse(tabs.shouldSeparateOrphans(state));
});
});
describe('tabForAnnotation', function () { describe('tabForAnnotation', function () {
unroll('shows annotation in correct tab', function (testCase) { unroll('shows annotation in correct tab', function (testCase) {
var ann = testCase.ann; var ann = testCase.ann;
var expectedTab = testCase.expectedTab; var expectedTab = testCase.expectedTab;
assert.equal(tabs.tabForAnnotation(ann, true /* separateOrphans */), expectedTab); assert.equal(tabs.tabForAnnotation(ann), expectedTab);
}, [{ }, [{
ann: fixtures.defaultAnnotation(), ann: fixtures.defaultAnnotation(),
expectedTab: uiConstants.TAB_ANNOTATIONS, expectedTab: uiConstants.TAB_ANNOTATIONS,
...@@ -49,58 +29,33 @@ describe('tabs', function () { ...@@ -49,58 +29,33 @@ describe('tabs', function () {
ann.$anchorTimeout = testCase.anchorTimeout; ann.$anchorTimeout = testCase.anchorTimeout;
ann.$orphan = testCase.orphan; ann.$orphan = testCase.orphan;
assert.equal(tabs.shouldShowInTab(ann, uiConstants.TAB_ANNOTATIONS, assert.equal(tabs.shouldShowInTab(ann, uiConstants.TAB_ANNOTATIONS), testCase.expectedTab === uiConstants.TAB_ANNOTATIONS);
testCase.separateOrphans), testCase.expectedTab === uiConstants.TAB_ANNOTATIONS); assert.equal(tabs.shouldShowInTab(ann, uiConstants.TAB_ORPHANS), testCase.expectedTab === uiConstants.TAB_ORPHANS);
assert.equal(tabs.shouldShowInTab(ann, uiConstants.TAB_ORPHANS,
testCase.separateOrphans), testCase.expectedTab === uiConstants.TAB_ORPHANS);
}, [{ }, [{
// Orphans tab disabled, anchoring in progress. // Anchoring in progress.
anchorTimeout: false,
orphan: undefined,
separateOrphans: false,
expectedTab: uiConstants.TAB_ANNOTATIONS,
},{
// Orphans tab disabled, anchoring succeeded.
anchorTimeout: false,
orphan: false,
separateOrphans: false,
expectedTab: uiConstants.TAB_ANNOTATIONS,
},{
// Orphans tab disabled, anchoring failed
anchorTimeout: false,
orphan: true,
separateOrphans: false,
expectedTab: uiConstants.TAB_ANNOTATIONS,
},{
// Orphans tab enabled, anchoring in progress.
anchorTimeout: false, anchorTimeout: false,
orphan: undefined, orphan: undefined,
separateOrphans: true,
expectedTab: null, expectedTab: null,
},{ },{
// Orphans tab enabled, anchoring succeeded. // Anchoring succeeded.
anchorTimeout: false, anchorTimeout: false,
orphan: false, orphan: false,
separateOrphans: true,
expectedTab: uiConstants.TAB_ANNOTATIONS, expectedTab: uiConstants.TAB_ANNOTATIONS,
},{ },{
// Orphans tab enabled, anchoring failed. // Anchoring failed.
anchorTimeout: false, anchorTimeout: false,
orphan: true, orphan: true,
separateOrphans: true,
expectedTab: uiConstants.TAB_ORPHANS, expectedTab: uiConstants.TAB_ORPHANS,
},{ },{
// Orphans tab enabled, anchoring timed out. // Anchoring timed out.
anchorTimeout: true, anchorTimeout: true,
orphan: undefined, orphan: undefined,
separateOrphans: true,
expectedTab: uiConstants.TAB_ANNOTATIONS, expectedTab: uiConstants.TAB_ANNOTATIONS,
},{ },{
// Orphans tab enabled, anchoring initially timed out but eventually // Anchoring initially timed out but eventually
// failed. // failed.
anchorTimeout: true, anchorTimeout: true,
orphan: true, orphan: true,
separateOrphans: true,
expectedTab: uiConstants.TAB_ORPHANS, expectedTab: uiConstants.TAB_ORPHANS,
}]); }]);
}); });
...@@ -109,16 +64,7 @@ describe('tabs', function () { ...@@ -109,16 +64,7 @@ describe('tabs', function () {
var annotation = Object.assign(fixtures.defaultAnnotation(), {$orphan:false}); var annotation = Object.assign(fixtures.defaultAnnotation(), {$orphan:false});
var orphan = Object.assign(fixtures.defaultAnnotation(), {$orphan:true}); var orphan = Object.assign(fixtures.defaultAnnotation(), {$orphan:true});
it('counts Annotations and Orphans together when the Orphans tab is not enabled', function () { it('counts Annotations and Orphans separately', function () {
assert.deepEqual(tabs.counts([annotation, orphan], false), {
anchoring: 0,
annotations: 2,
notes: 0,
orphans: 0,
});
});
it('counts Annotations and Orphans separately when the Orphans tab is enabled', function () {
assert.deepEqual(tabs.counts([annotation, orphan], true), { assert.deepEqual(tabs.counts([annotation, orphan], true), {
anchoring: 0, anchoring: 0,
annotations: 1, annotations: 1,
......
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