From 46059f3ad3d2ebbd0b82f6015d86b9caf6cbf5fa Mon Sep 17 00:00:00 2001 From: bio-boris Date: Wed, 7 Oct 2020 16:42:07 -0500 Subject: [PATCH 1/4] Moves button. Comments out old section, not sure what that space was for --- .../narrative_core/kbaseCellToolbarMenu.js | 50 ++++++++++--------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js b/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js index 7523f6391c..c604e8469c 100644 --- a/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js +++ b/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js @@ -368,7 +368,32 @@ define([ var events = Events.make({ node: container }), buttons = [ div({ class: 'buttons pull-right' }, [ - span({ class: 'kb-func-timestamp' }), + renderOptions(cell, events), + (function() { + var toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'), + toggleIcon = (toggleMinMax === 'maximized' ? 'minus' : 'plus'), + color = (toggleMinMax === 'maximized' ? '#000' : 'rgba(255,137,0,1)'); + return button({ + type: 'button', + class: 'btn btn-default btn-xs', + dataToggle: 'tooltip', + dataPlacement: 'left', + title: true, + dataOriginalTitle: toggleMinMax === 'maximized' ? 'Collapse Cell' : 'Expand Cell', + id: events.addEvent({ type: 'click', handler: doToggleMinMaxCell }) + }, [ + span({ + class: 'fa fa-' + toggleIcon + '-square-o fa-lg', + style: { + color: color + } + }) + ]); + }()), + + + + // span({ class: 'kb-func-timestamp' }), span({ class: 'fa fa-circle-o-notch fa-spin', style: { color: 'rgb(42, 121, 191)', display: 'none' } }), span({ class: 'fa fa-exclamation-triangle', style: { color: 'rgb(255, 0, 0)', display: 'none' } }), (readOnly ? null : button({ @@ -393,28 +418,7 @@ define([ }, [ span({ class: 'fa fa-arrow-down fa-lg' }) ])), - renderOptions(cell, events), - (function() { - var toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'), - toggleIcon = (toggleMinMax === 'maximized' ? 'minus' : 'plus'), - color = (toggleMinMax === 'maximized' ? '#000' : 'rgba(255,137,0,1)'); - return button({ - type: 'button', - class: 'btn btn-default btn-xs', - dataToggle: 'tooltip', - dataPlacement: 'left', - title: true, - dataOriginalTitle: toggleMinMax === 'maximized' ? 'Collapse Cell' : 'Expand Cell', - id: events.addEvent({ type: 'click', handler: doToggleMinMaxCell }) - }, [ - span({ - class: 'fa fa-' + toggleIcon + '-square-o fa-lg', - style: { - color: color - } - }) - ]); - }()) + ]) ], message = div({ From fca5adbe812293194994a2d33f2f14f3e8e66841 Mon Sep 17 00:00:00 2001 From: bio-boris Date: Wed, 7 Oct 2020 17:14:07 -0500 Subject: [PATCH 2/4] Fixes order of buttons --- .../narrative_core/kbaseCellToolbarMenu.js | 46 +++++++++---------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js b/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js index c604e8469c..3272fe006c 100644 --- a/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js +++ b/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js @@ -369,31 +369,6 @@ define([ buttons = [ div({ class: 'buttons pull-right' }, [ renderOptions(cell, events), - (function() { - var toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'), - toggleIcon = (toggleMinMax === 'maximized' ? 'minus' : 'plus'), - color = (toggleMinMax === 'maximized' ? '#000' : 'rgba(255,137,0,1)'); - return button({ - type: 'button', - class: 'btn btn-default btn-xs', - dataToggle: 'tooltip', - dataPlacement: 'left', - title: true, - dataOriginalTitle: toggleMinMax === 'maximized' ? 'Collapse Cell' : 'Expand Cell', - id: events.addEvent({ type: 'click', handler: doToggleMinMaxCell }) - }, [ - span({ - class: 'fa fa-' + toggleIcon + '-square-o fa-lg', - style: { - color: color - } - }) - ]); - }()), - - - - // span({ class: 'kb-func-timestamp' }), span({ class: 'fa fa-circle-o-notch fa-spin', style: { color: 'rgb(42, 121, 191)', display: 'none' } }), span({ class: 'fa fa-exclamation-triangle', style: { color: 'rgb(255, 0, 0)', display: 'none' } }), (readOnly ? null : button({ @@ -419,6 +394,27 @@ define([ span({ class: 'fa fa-arrow-down fa-lg' }) ])), + (function() { + var toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'), + toggleIcon = (toggleMinMax === 'maximized' ? 'minus' : 'plus'), + color = (toggleMinMax === 'maximized' ? '#000' : 'rgba(255,137,0,1)'); + return button({ + type: 'button', + class: 'btn btn-default btn-xs', + dataToggle: 'tooltip', + dataPlacement: 'left', + title: true, + dataOriginalTitle: toggleMinMax === 'maximized' ? 'Collapse Cell' : 'Expand Cell', + id: events.addEvent({ type: 'click', handler: doToggleMinMaxCell }) + }, [ + span({ + class: 'fa fa-' + toggleIcon + '-square-o fa-lg', + style: { + color: color + } + }) + ]); + }()) ]) ], message = div({ From 35ea95139ca3929d2bfe824dec5c1b98e9fade2b Mon Sep 17 00:00:00 2001 From: bio-boris Date: Thu, 15 Oct 2020 00:21:42 -0500 Subject: [PATCH 3/4] Add data-test attribute to test order of cell buttons --- .../narrative_core/kbaseCellToolbarMenu.js | 6 ++- .../kbaseCellToolbarMenu-spec.js | 51 +++++++++++++++---- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js b/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js index 3272fe006c..0864f76d7d 100644 --- a/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js +++ b/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js @@ -279,7 +279,8 @@ define([ id: dropdownId, dataToggle: 'dropdown', ariaHaspopup: 'true', - ariaExpanded: 'true' + ariaExpanded: 'true', + 'data-test' : 'cell-dropdown' }, [span({ class: 'fa fa-ellipsis-h fa-lg' })]), ul({ class: 'dropdown-menu dropdown-menu-right', @@ -378,6 +379,7 @@ define([ dataPlacement: 'left', title: true, dataOriginalTitle: 'Move Cell Up', + 'data-test' : 'cell-move-up', id: events.addEvent({ type: 'click', handler: doMoveCellUp }) }, [ span({ class: 'fa fa-arrow-up fa-lg' }) @@ -389,6 +391,7 @@ define([ dataPlacement: 'left', title: true, dataOriginalTitle: 'Move Cell Down', + 'data-test' : 'cell-move-down', id: events.addEvent({ type: 'click', handler: doMoveCellDown }) }, [ span({ class: 'fa fa-arrow-down fa-lg' }) @@ -404,6 +407,7 @@ define([ dataToggle: 'tooltip', dataPlacement: 'left', title: true, + 'data-test' : 'cell-toggle-expansion', dataOriginalTitle: toggleMinMax === 'maximized' ? 'Collapse Cell' : 'Expand Cell', id: events.addEvent({ type: 'click', handler: doToggleMinMaxCell }) }, [ diff --git a/test/unit/spec/narrative_core/kbaseCellToolbarMenu-spec.js b/test/unit/spec/narrative_core/kbaseCellToolbarMenu-spec.js index db5e9568c8..05ed11f49b 100644 --- a/test/unit/spec/narrative_core/kbaseCellToolbarMenu-spec.js +++ b/test/unit/spec/narrative_core/kbaseCellToolbarMenu-spec.js @@ -1,14 +1,16 @@ /*global describe, it, expect, beforeAll */ /*jslint white: true*/ define([ + 'jquery', 'kbaseCellToolbarMenu', 'base/js/namespace' -], function( +], function ( + $, Widget, Jupyter ) { 'use strict'; - describe('Test the kbaseCellToolbarMenu widget', function() { + describe('Test the kbaseCellToolbarMenu widget', function () { beforeAll(() => { Jupyter.narrative = { readonly: false @@ -40,6 +42,7 @@ define([ } }; }; + const mockToolbar = (mode, stage, collapsedState) => { const instance = Widget.make(); const parentCell = mockParentCell(mode, stage, collapsedState); @@ -50,49 +53,77 @@ define([ )[0].innerText; }; - it('Should say Error when minimized and mode is error', function() { + + const mockToolbarDataTestNodes = (mode, stage, collapsedState) => { + const instance = Widget.make(); + const parentCell = mockParentCell(mode, stage, collapsedState); + const toolbarDiv = document.createElement('div'); + instance.register_callback([toolbarDiv], parentCell); + return toolbarDiv.querySelectorAll( + '[data-test]' + ); + }; + + // This test might better be served through Snapshot Testing + // This test might want to check to see if each buttton has the correct fa-* class + it('Should render the correct app cell buttons in the correct order', function () { + var testToolBar = mockToolbarDataTestNodes('success', '', 'maximized'); + var expectedButtonOrder = ['cell-dropdown', 'cell-move-up', 'cell-move-down', 'cell-toggle-expansion']; + var extractedButtons = []; + testToolBar.forEach(function (element) { + var attribute = element.getAttribute('data-test'); + if (expectedButtonOrder.includes(attribute)) { + extractedButtons.push(attribute); + } + }); + expect(extractedButtons.length).toEqual(expectedButtonOrder.length); + expect(extractedButtons).toEqual(expectedButtonOrder); + }); + + + it('Should say Error when minimized and mode is error', function () { expect( mockToolbar('error', '', 'minimized') ).toBe('Error'); }); - it('Should say Error when minimized and mode is internal-error', function() { + it('Should say Error when minimized and mode is internal-error', function () { expect( mockToolbar('internal-error', '', 'minimized') ).toBe('Error'); }); - it('Should say Canceled when minimized and canceling', function() { + it('Should say Canceled when minimized and canceling', function () { expect( mockToolbar('canceling', '', 'minimized') ).toBe('Canceled'); }); - it('Should say Canceled when minimized and canceled', function() { + it('Should say Canceled when minimized and canceled', function () { expect( mockToolbar('canceled', '', 'minimized') ).toBe('Canceled'); }); - it('Should say Running when minimized and running', function() { + it('Should say Running when minimized and running', function () { expect( mockToolbar('processing', 'running', 'minimized') ).toBe('Running'); }); - it('Should say Running when minimized and queued', function() { + it('Should say Running when minimized and queued', function () { expect( mockToolbar('processing', 'queued', 'minimized') ).toBe('Queued'); }); - it('Should say Success when minimized and mode is success', function() { + it('Should say Success when minimized and mode is success', function () { expect( mockToolbar('success', '', 'minimized') ).toBe('Success'); }); - it('Should suppress the status message if maximized', function() { + it('Should suppress the status message if maximized', function () { expect( mockToolbar('processing', 'running', 'maximized') ).toBe(''); From cc0c0ddee8f2137df8f86228926077a19926cde6 Mon Sep 17 00:00:00 2001 From: bio-boris Date: Thu, 15 Oct 2020 00:37:21 -0500 Subject: [PATCH 4/4] Added voiceover labels --- .../narrative_core/kbaseCellToolbarMenu.js | 144 ++++++++++-------- 1 file changed, 78 insertions(+), 66 deletions(-) diff --git a/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js b/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js index 0864f76d7d..97d559805c 100644 --- a/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js +++ b/kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js @@ -10,7 +10,7 @@ define([ 'kbase/js/widgets/appInfoPanel', 'narrativeConfig', 'custom/custom' -], function( +], function ( $, html, Events, @@ -88,14 +88,14 @@ define([ if (url) { return a({ - href: url, - target: '_blank', - id: events.addEvent({ - type: 'click', - handler: doShowInfoModal - }) - }, - label || 'ref'); + href: url, + target: '_blank', + id: events.addEvent({ + type: 'click', + handler: doShowInfoModal + }) + }, + label || 'ref'); } return ''; } @@ -134,7 +134,7 @@ define([ body: $('
'), buttons: [ $('View on App Store'), - $('').click(function() { + $('').click(function () { dialog.hide(); }) ], @@ -149,9 +149,9 @@ define([ appModule: module, tag: tag }); - infoPanel.start({ node: dialog.getBody() }); + infoPanel.start({node: dialog.getBody()}); - dialog.getElement().on('hidden.bs.modal', function() { + dialog.getElement().on('hidden.bs.modal', function () { dialog.destroy(); }); dialog.show(); @@ -174,16 +174,16 @@ define([ dataPlacement: 'left', title: true, dataOriginalTitle: 'Cell Settings', - id: events.addEvent({ type: 'click', handler: doToggleCellSettings }) + id: events.addEvent({type: 'click', handler: doToggleCellSettings}) }, [ - span({ class: 'fa fa-cog', style: 'font-size: 14pt' }) + span({class: 'fa fa-cog', style: 'font-size: 14pt'}) ]); } function renderIcon(icon) { return span({ class: 'fa fa-' + icon.type + ' fa-sm', - style: { color: icon.color || '#000' } + style: {color: icon.color || '#000'} }); } @@ -202,8 +202,7 @@ define([ var toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'), toggleIcon = (toggleMinMax === 'maximized' ? 'minus' : 'plus'), dropdownId = html.genId(), - menuItems = [ - ]; + menuItems = []; if (cell.cell_type === 'code') { menuItems.push({ @@ -213,7 +212,7 @@ define([ type: 'terminal', color: 'black' }, - id: events.addEvent({ type: 'click', handler: doToggleCodeView }) + id: events.addEvent({type: 'click', handler: doToggleCodeView}) }); } @@ -227,7 +226,7 @@ define([ }, id: events.addEvent({ type: 'click', - handler: function() { + handler: function () { cell.showInfo(); } }) @@ -264,7 +263,7 @@ define([ type: 'times', color: 'red' }, - id: events.addEvent({ type: 'click', handler: doDeleteCell }) + id: events.addEvent({type: 'click', handler: doDeleteCell}) }); } @@ -272,7 +271,7 @@ define([ return ''; } - return span({ class: 'dropdown' }, [ + return span({class: 'dropdown'}, [ button({ class: 'btn btn-xs btn-default dropdown-toggle', type: 'button', @@ -280,13 +279,14 @@ define([ dataToggle: 'dropdown', ariaHaspopup: 'true', ariaExpanded: 'true', - 'data-test' : 'cell-dropdown' - }, [span({ class: 'fa fa-ellipsis-h fa-lg' })]), + 'aria-label': 'cell options', + 'data-test': 'cell-dropdown' + }, [span({class: 'fa fa-ellipsis-h fa-lg'})]), ul({ class: 'dropdown-menu dropdown-menu-right', ariaLabelledby: dropdownId }, [ - menuItems.map(function(item) { + menuItems.map(function (item) { switch (item.type) { case 'separator': return li({ @@ -331,19 +331,19 @@ define([ } function minimizedStatus(mode, stage) { - if(mode === 'error' || mode === 'internal-error') { + if (mode === 'error' || mode === 'internal-error') { return 'Error'; } - if(mode === 'canceled' || mode === 'canceling') { + if (mode === 'canceled' || mode === 'canceling') { return 'Canceled'; } - if(mode ==='processing' && stage === 'running') { + if (mode === 'processing' && stage === 'running') { return 'Running'; } - if(mode ==='processing' && stage === 'queued') { + if (mode === 'processing' && stage === 'queued') { return 'Queued'; } - if(mode === 'success') { + if (mode === 'success') { return 'Success'; } return ''; @@ -366,12 +366,15 @@ define([ const appStatePretty = minimizedStatus(fsmMode, fsmStage); const collapsedCellStatus = cellCollapsed ? appStatePretty : ''; - var events = Events.make({ node: container }), + var events = Events.make({node: container}), buttons = [ - div({ class: 'buttons pull-right' }, [ + div({class: 'buttons pull-right'}, [ renderOptions(cell, events), - span({ class: 'fa fa-circle-o-notch fa-spin', style: { color: 'rgb(42, 121, 191)', display: 'none' } }), - span({ class: 'fa fa-exclamation-triangle', style: { color: 'rgb(255, 0, 0)', display: 'none' } }), + span({ + class: 'fa fa-circle-o-notch fa-spin', + style: {color: 'rgb(42, 121, 191)', display: 'none'} + }), + span({class: 'fa fa-exclamation-triangle', style: {color: 'rgb(255, 0, 0)', display: 'none'}}), (readOnly ? null : button({ type: 'button', class: 'btn btn-default btn-xs', @@ -379,10 +382,11 @@ define([ dataPlacement: 'left', title: true, dataOriginalTitle: 'Move Cell Up', - 'data-test' : 'cell-move-up', - id: events.addEvent({ type: 'click', handler: doMoveCellUp }) + 'data-test': 'cell-move-up', + 'aria-label': 'Move cell up', + id: events.addEvent({type: 'click', handler: doMoveCellUp}) }, [ - span({ class: 'fa fa-arrow-up fa-lg' }) + span({class: 'fa fa-arrow-up fa-lg'}) ])), (readOnly ? null : button({ type: 'button', @@ -391,13 +395,14 @@ define([ dataPlacement: 'left', title: true, dataOriginalTitle: 'Move Cell Down', - 'data-test' : 'cell-move-down', - id: events.addEvent({ type: 'click', handler: doMoveCellDown }) + 'data-test': 'cell-move-down', + 'aria-label': 'Move cell down', + id: events.addEvent({type: 'click', handler: doMoveCellDown}) }, [ - span({ class: 'fa fa-arrow-down fa-lg' }) + span({class: 'fa fa-arrow-down fa-lg'}) ])), - (function() { + (function () { var toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'), toggleIcon = (toggleMinMax === 'maximized' ? 'minus' : 'plus'), color = (toggleMinMax === 'maximized' ? '#000' : 'rgba(255,137,0,1)'); @@ -407,9 +412,10 @@ define([ dataToggle: 'tooltip', dataPlacement: 'left', title: true, - 'data-test' : 'cell-toggle-expansion', + 'data-test': 'cell-toggle-expansion', + 'aria-label': 'Expand or Collapse Cell', dataOriginalTitle: toggleMinMax === 'maximized' ? 'Collapse Cell' : 'Expand Cell', - id: events.addEvent({ type: 'click', handler: doToggleMinMaxCell }) + id: events.addEvent({type: 'click', handler: doToggleMinMaxCell}) }, [ span({ class: 'fa fa-' + toggleIcon + '-square-o fa-lg', @@ -431,20 +437,24 @@ define([ utils.getCellMeta(cell, 'kbase.cellState.message') ]) ]), - content = div({ class: 'kb-cell-toolbar' }, [ - div({ class: '', style: { - display: 'flex', - flexDirection: 'row', - height: '56px', - justifyContent: 'space-between', - } }, [ + content = div({class: 'kb-cell-toolbar'}, [ + div({ + class: '', style: { + display: 'flex', + flexDirection: 'row', + height: '56px', + justifyContent: 'space-between', + } + }, [ div({ class: 'title-container', style: {flexGrow: '1'} }, [ - div({ class: 'title', style: { - display: 'flex', height: '56px' - } }, [ + div({ + class: 'title', style: { + display: 'flex', height: '56px' + } + }, [ div({ dataElement: 'icon', class: 'icon', @@ -457,7 +467,7 @@ define([ }, [ buildIcon(cell) ]), - div({ style: { flexGrow: '1' } }, [ + div({style: {flexGrow: '1'}}, [ div({ dataElement: 'title', class: 'title', @@ -482,17 +492,19 @@ define([ }, [getCellSubtitle(cell)]) ]), div( - { style: { - margin: '0px 0px 0px auto', - minWidth: '65px' - }}, + { + style: { + margin: '0px 0px 0px auto', + minWidth: '65px' + } + }, [collapsedCellStatus] ) ]) ]), div({ class: 'buttons-container', - style: { minWidth: '110px' } + style: {minWidth: '110px'} }, [ buttons, message @@ -519,13 +531,13 @@ define([ role: 'button', title: 'New version available', dataContent: 'This app has a newer version available! ' + - 'There\'s probably nothing wrong with this version, ' + - 'but the new one may include new features. Add a new "' + - utils.getCellMeta(cell, 'kbase.appCell.newAppName') + - '" app cell for the update.', - style: { color: '#f79b22' } + 'There\'s probably nothing wrong with this version, ' + + 'but the new one may include new features. Add a new "' + + utils.getCellMeta(cell, 'kbase.appCell.newAppName') + + '" app cell for the update.', + style: {color: '#f79b22'} }, [ - span({ class: 'fa fa-exclamation-triangle fa-lg' }) + span({class: 'fa fa-exclamation-triangle fa-lg'}) ]); } else { return ''; @@ -543,7 +555,7 @@ define([ rendered.events.attachEvents(); // try this... - container.addEventListener('dblclick', function(e) { + container.addEventListener('dblclick', function (e) { doToggleMinMaxCell(e); }); } catch (ex) { @@ -557,7 +569,7 @@ define([ } return { - make: function(config) { + make: function (config) { return factory(config); } };