From e1c7149c6290ca08164895fc64ffbe70272b7b5e Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 6 Apr 2020 17:37:27 -0700 Subject: [PATCH 1/4] remove jquery from job log viewer, other minor fixes --- .../static/kbase/js/util/jobLogViewer.js | 91 +++++++++---------- 1 file changed, 44 insertions(+), 47 deletions(-) diff --git a/kbase-extension/static/kbase/js/util/jobLogViewer.js b/kbase-extension/static/kbase/js/util/jobLogViewer.js index a57e5e410a..1ff84f23de 100644 --- a/kbase-extension/static/kbase/js/util/jobLogViewer.js +++ b/kbase-extension/static/kbase/js/util/jobLogViewer.js @@ -8,7 +8,6 @@ define([ 'common/events', 'common/fsm', 'kb_common/html', - 'jquery', 'css!kbase/css/kbaseJobLog.css' ], function( Promise, @@ -17,12 +16,11 @@ define([ UI, Events, Fsm, - html, - $ + html ) { 'use strict'; - var t = html.tag, + let t = html.tag, div = t('div'), button = t('button'), span = t('span'), @@ -338,6 +336,7 @@ define([ bus = runtime.bus().makeChannelBus({ description: 'Log Viewer Bus' }), container, jobId, + panelId, model, ui, startingLine = 0, @@ -414,7 +413,7 @@ define([ } function requestLatestJobLog() { - // only while job is running + // only while job is running // load numLines at a time // otherwise load entire log let autoState = fsm.getCurrentState().state.auto; @@ -455,31 +454,27 @@ define([ }) } - function test(){ - if(panelHeight === smallPanelHeight){ - panelHeight = largePanelHeight; - }else{ - panelHeight = smallPanelHeight; - } - $(ui.getElements('panel')[0]).animate({height: panelHeight}, 500); + function toggleViewerSize() { + panelHeight = panelHeight === smallPanelHeight ? largePanelHeight : smallPanelHeight; + getPanelNode().style.height = panelHeight; } // VIEW /** * builds contents of panel-heading div - * @param {??} events + * @param {??} events */ - function renderControls(events) { + function renderControls(events) { return div({ dataElement: 'header', style: { margin: '0 0 10px 0' } }, [ button({ class: 'btn btn-sm btn-default', dataButton: 'expand', dataToggle: 'tooltip', dataPlacement: 'top', - title: 'Start fetching logs', + title: 'Toggle log viewer size', id: events.addEvent({ type: 'click', - handler: test + handler: toggleViewerSize }) }, [ span({ class: 'fa fa-expand' }) @@ -545,10 +540,10 @@ define([ /** * builds contents of panel-body class - * @param {number} jobId + * @param {string} panelId */ - function renderLayout(jobId) { - var events = Events.make(), + function renderLayout(panelId) { + const events = Events.make(), content = div({ dataElement: 'kb-log', style: { marginTop: '10px'}}, [ div({ class: 'kblog-header' }, [ div({ class: 'kblog-num-wrapper' }, [ @@ -558,10 +553,13 @@ define([ renderControls(events) // header ]) ]), - div({ dataElement: 'panel', class: jobId, + div({ dataElement: 'panel', id: panelId, style: { - 'overflow-y': 'scroll', height: panelHeight - } }) + 'overflow-y': 'scroll', + height: panelHeight, + transition: 'height 0.5s' + } + }) ]); return { @@ -597,7 +595,7 @@ define([ * foobarbaz * * - * @param {object} line + * @param {object} line */ function buildLine(line) { // kblog-line wrapper div @@ -605,23 +603,23 @@ define([ const kblogLine = document.createElement('div') kblogLine.setAttribute('class', 'kblog-line' + errorClass); // kblog-num-wrapper div - const warpperDiv = document.createElement('div'); - warpperDiv.setAttribute('class', 'kblog-num-wrapper'); - // number + const wrapperDiv = document.createElement('div'); + wrapperDiv.setAttribute('class', 'kblog-num-wrapper'); + // number const numSpan = document.createElement('span'); numSpan.setAttribute('class', 'kblog-line-num'); const lineNumber = document.createTextNode(line.lineNumber); numSpan.appendChild(lineNumber); - // text + // text const textSpan = document.createElement('span'); textSpan.setAttribute('class', 'kblog-text'); const lineText = document.createTextNode(line.text) textSpan.appendChild(lineText); - // append line number and text - warpperDiv.appendChild(numSpan); - warpperDiv.appendChild(textSpan); + // append line number and text + wrapperDiv.appendChild(numSpan); + wrapperDiv.appendChild(textSpan); // append wrapper to line div - kblogLine.appendChild(warpperDiv) + kblogLine.appendChild(wrapperDiv) return kblogLine; } @@ -629,7 +627,7 @@ define([ /** * Append div that displays job log lines * to the panel - * @param {array} lines + * @param {array} lines */ function makeLogChunkDiv(lines) { for (let i=0; i Date: Mon, 6 Apr 2020 17:47:32 -0700 Subject: [PATCH 2/4] add initial test spec for job log viewer unit testing --- test/unit/spec/Util/jobLogViewerSpec.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 test/unit/spec/Util/jobLogViewerSpec.js diff --git a/test/unit/spec/Util/jobLogViewerSpec.js b/test/unit/spec/Util/jobLogViewerSpec.js new file mode 100644 index 0000000000..edc1d8fb95 --- /dev/null +++ b/test/unit/spec/Util/jobLogViewerSpec.js @@ -0,0 +1,22 @@ +/*global define, describe, it, expect, jasmine, beforeEach, afterEach*/ +/*jslint white: true*/ +define([ + 'util/jobLogViewer' +], ( + JobLogViewer +) => { + describe('Test the job log viewer module', () => { + beforeEach(() => { + + }); + + it('Should load the module code successfully', () => { + expect(JobLogViewer).toBeDefined(); + }); + + it('Should have the factory method', () => { + expect(JobLogViewer.make).toBeDefined(); + expect(JobLogViewer.make).toEqual(jasmine.any(Function)); + }); + }); +}) From e1ee3e76a501009a994558cb7e7842a5cf4a5fb6 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Tue, 7 Apr 2020 11:37:47 -0700 Subject: [PATCH 3/4] issue 1620 - format log lines more cleanly --- .../static/kbase/css/kbaseJobLog.css | 10 ++--- .../static/kbase/js/util/jobLogViewer.js | 38 ++++++++++++------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/kbase-extension/static/kbase/css/kbaseJobLog.css b/kbase-extension/static/kbase/css/kbaseJobLog.css index a3dc1627b5..3da3e65501 100644 --- a/kbase-extension/static/kbase/css/kbaseJobLog.css +++ b/kbase-extension/static/kbase/css/kbaseJobLog.css @@ -16,13 +16,11 @@ .kblog-header { display: flex; font-family: monospace; - font-size: 85%; + font-size: 85%; } .kblog-line { - display: flex; font-family: monospace; - font-size: 85%; } .kblog-line:hover { @@ -31,11 +29,13 @@ .kblog-num-wrapper { font-size: 85%; + display: flex; + flex-direction: row; } .kblog-line-num { flex-shrink: 0; - width: 9ex; + width: 3rem; text-align: right; color: #999; white-space: nowrap; @@ -72,4 +72,4 @@ #kblog-err { margin-top: 5px; /* color: #660000; */ -} \ No newline at end of file +} diff --git a/kbase-extension/static/kbase/js/util/jobLogViewer.js b/kbase-extension/static/kbase/js/util/jobLogViewer.js index 1ff84f23de..2465277134 100644 --- a/kbase-extension/static/kbase/js/util/jobLogViewer.js +++ b/kbase-extension/static/kbase/js/util/jobLogViewer.js @@ -330,9 +330,13 @@ define([ } ]; - function factory(config) { - var config = config || {}, - runtime = Runtime.make(), + /** + * The entrypoint to this widget. This creates the job log viewer and initializes it. + * Starting it is left as a lifecycle method for the calling object. + * + */ + function factory() { + let runtime = Runtime.make(), bus = runtime.bus().makeChannelBus({ description: 'Log Viewer Bus' }), container, jobId, @@ -606,20 +610,20 @@ define([ const wrapperDiv = document.createElement('div'); wrapperDiv.setAttribute('class', 'kblog-num-wrapper'); // number - const numSpan = document.createElement('span'); - numSpan.setAttribute('class', 'kblog-line-num'); + const numDiv = document.createElement('div'); + numDiv.setAttribute('class', 'kblog-line-num'); const lineNumber = document.createTextNode(line.lineNumber); - numSpan.appendChild(lineNumber); + numDiv.appendChild(lineNumber); // text - const textSpan = document.createElement('span'); - textSpan.setAttribute('class', 'kblog-text'); + const textDiv = document.createElement('div'); + textDiv.setAttribute('class', 'kblog-text'); const lineText = document.createTextNode(line.text) - textSpan.appendChild(lineText); + textDiv.appendChild(lineText); // append line number and text - wrapperDiv.appendChild(numSpan); - wrapperDiv.appendChild(textSpan); + wrapperDiv.appendChild(numDiv); + wrapperDiv.appendChild(textDiv); // append wrapper to line div - kblogLine.appendChild(wrapperDiv) + kblogLine.appendChild(wrapperDiv); return kblogLine; } @@ -1025,6 +1029,14 @@ define([ return document.getElementById(panelId); } + /** + * The main lifecycle event, called when its container node exists, and we want to start + * running this widget. + * This detaches itself first, if it exists, then recreates itself in its host node. + * @param {object} arg - should have attributes: + * - node - a DOM node where it will be hosted. + * - jobId - string, a job id for this log + */ function start(arg) { detach(); // if we're alive, remove ourselves before restarting var hostNode = arg.node; @@ -1098,7 +1110,7 @@ define([ return { make: function(config) { - return factory(config); + return factory(); } }; }); From 0506272cd9921ff1bac42e8e4903520c23a53dee Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Tue, 7 Apr 2020 15:27:16 -0700 Subject: [PATCH 4/4] add testing, test for startup fails --- .../static/kbase/js/util/jobLogViewer.js | 19 ++++++++- test/unit/spec/Util/jobLogViewerSpec.js | 42 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/kbase-extension/static/kbase/js/util/jobLogViewer.js b/kbase-extension/static/kbase/js/util/jobLogViewer.js index 2465277134..cd61d15d97 100644 --- a/kbase-extension/static/kbase/js/util/jobLogViewer.js +++ b/kbase-extension/static/kbase/js/util/jobLogViewer.js @@ -1,5 +1,13 @@ /*global define*/ /*jslint white:true,browser:true*/ +/** + * Usage: + * let viewer = JobLogViewer.make(); + * viewer.start({ + * jobId: , + * node: + * }) + */ define([ 'bluebird', 'common/runtime', @@ -1040,10 +1048,17 @@ define([ function start(arg) { detach(); // if we're alive, remove ourselves before restarting var hostNode = arg.node; + if (!hostNode) { + throw new Error('Requires a node to start'); + } + jobId = arg.jobId; + if (!jobId) { + throw new Error('Requires a job id to start'); + } + container = hostNode.appendChild(document.createElement('div')); ui = UI.make({ node: container }); - jobId = arg.jobId; panelId = html.genId(); var layout = renderLayout(panelId); container.innerHTML = layout.content; @@ -1079,7 +1094,7 @@ define([ function detach() { stop(); if (container) { - container.innerHTML = ''; + container.remove(); } } diff --git a/test/unit/spec/Util/jobLogViewerSpec.js b/test/unit/spec/Util/jobLogViewerSpec.js index edc1d8fb95..3fba8bbd4d 100644 --- a/test/unit/spec/Util/jobLogViewerSpec.js +++ b/test/unit/spec/Util/jobLogViewerSpec.js @@ -6,8 +6,14 @@ define([ JobLogViewer ) => { describe('Test the job log viewer module', () => { + let hostNode = null; beforeEach(() => { + hostNode = document.createElement('div'); + document.body.appendChild(hostNode); + }); + afterEach(() => { + hostNode.remove(); }); it('Should load the module code successfully', () => { @@ -18,5 +24,41 @@ define([ expect(JobLogViewer.make).toBeDefined(); expect(JobLogViewer.make).toEqual(jasmine.any(Function)); }); + + it('Should be created', () => { + let viewer = JobLogViewer.make(); + expect(viewer.start).toEqual(jasmine.any(Function)); + expect(viewer.stop).toEqual(jasmine.any(Function)); + expect(viewer.detach).toEqual(jasmine.any(Function)); + }); + + it('Should fail to start without a node', () => { + let viewer = JobLogViewer.make(); + const jobId = 'fakejob'; + let arg = { + jobId: jobId + }; + expect(() => {viewer.start(arg)}).toThrow(new Error('Requires a node to start')); + }); + + it('Should fail to start without a jobId', () => { + let viewer = JobLogViewer.make(); + let arg = { + node: hostNode + }; + expect(() => {viewer.start(arg)}).toThrow(new Error('Requires a job id to start')); + }); + + it('Should start as expected with inputs, and be stoppable and detachable', () => { + let viewer = JobLogViewer.make(); + let arg = { + node: hostNode, + jobId: 'someFakeJob' + }; + viewer.start(arg); + expect(hostNode.querySelector('div[data-element="kb-log"]')).toBeDefined(); + viewer.detach(); + expect(hostNode.innerHTML).toBe(''); + }); }); })