From 95da2b112f71442a73b4a1650c4b572ba8d5efb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 30 Dec 2019 15:44:22 -0500 Subject: [PATCH 1/6] fix `logging` config option declaration - set valType to 'integer' - set min and max --- src/plot_api/plot_config.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plot_api/plot_config.js b/src/plot_api/plot_config.js index 2f2f750332b..cdf3bab84f3 100644 --- a/src/plot_api/plot_config.js +++ b/src/plot_api/plot_config.js @@ -376,7 +376,9 @@ var configAttributes = { }, logging: { - valType: 'boolean', + valType: 'integer', + min: 0, + max: 2, dflt: 1, description: [ 'Turn all console logging on or off (errors will be thrown)', From 3547173dd7e4f5b778e9eda73594b9d4bad9e175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 30 Dec 2019 15:45:48 -0500 Subject: [PATCH 2/6] add onGraphLogging config option - add option to notifier such that it stays on-graph w/o fading out - within lib/loggers, call notifier with log/warn/error messages --- src/lib/loggers.js | 44 +++++++++++++++++++++++------ src/lib/notifier.js | 18 ++++++++---- src/plot_api/plot_config.js | 15 ++++++++++ test/jasmine/tests/lib_test.js | 51 ++++++++++++++++++++++++++++++++-- 4 files changed, 111 insertions(+), 17 deletions(-) diff --git a/src/lib/loggers.js b/src/lib/loggers.js index 3524ac2dd27..56a1c863a8f 100644 --- a/src/lib/loggers.js +++ b/src/lib/loggers.js @@ -12,6 +12,8 @@ var dfltConfig = require('../plot_api/plot_config').dfltConfig; +var notifier = require('./notifier'); + var loggers = module.exports = {}; /** @@ -21,39 +23,63 @@ var loggers = module.exports = {}; */ loggers.log = function() { + var i; + if(dfltConfig.logging > 1) { var messages = ['LOG:']; - - for(var i = 0; i < arguments.length; i++) { + for(i = 0; i < arguments.length; i++) { messages.push(arguments[i]); } - apply(console.trace || console.log, messages); } + + if(dfltConfig.onGraphLogging > 1) { + var lines = []; + for(i = 0; i < arguments.length; i++) { + lines.push(arguments[i]); + } + notifier(lines.join('
'), 'long'); + } }; loggers.warn = function() { + var i; + if(dfltConfig.logging > 0) { var messages = ['WARN:']; - - for(var i = 0; i < arguments.length; i++) { + for(i = 0; i < arguments.length; i++) { messages.push(arguments[i]); } - apply(console.trace || console.log, messages); } + + if(dfltConfig.onGraphLogging > 0) { + var lines = []; + for(i = 0; i < arguments.length; i++) { + lines.push(arguments[i]); + } + notifier(lines.join('
'), 'stick'); + } }; loggers.error = function() { + var i; + if(dfltConfig.logging > 0) { var messages = ['ERROR:']; - - for(var i = 0; i < arguments.length; i++) { + for(i = 0; i < arguments.length; i++) { messages.push(arguments[i]); } - apply(console.error, messages); } + + if(dfltConfig.onGraphLogging > 0) { + var lines = []; + for(i = 0; i < arguments.length; i++) { + lines.push(arguments[i]); + } + notifier(lines.join('
'), 'stick'); + } }; /* diff --git a/src/lib/notifier.js b/src/lib/notifier.js index 34d3fe47dfa..5a7689eea21 100644 --- a/src/lib/notifier.js +++ b/src/lib/notifier.js @@ -70,11 +70,17 @@ module.exports = function(text, displayLength) { p.append('span').text(lines[i]); } - note.transition() - .duration(700) - .style('opacity', 1) - .transition() - .delay(ts) - .call(killNote); + if(displayLength === 'stick') { + note.transition() + .duration(700) + .style('opacity', 1); + } else { + note.transition() + .duration(700) + .style('opacity', 1) + .transition() + .delay(ts) + .call(killNote); + } }); }; diff --git a/src/plot_api/plot_config.js b/src/plot_api/plot_config.js index cdf3bab84f3..309e0c708ce 100644 --- a/src/plot_api/plot_config.js +++ b/src/plot_api/plot_config.js @@ -390,6 +390,21 @@ var configAttributes = { ].join(' ') }, + onGraphLogging: { + valType: 'integer', + min: 0, + max: 2, + dflt: 0, + description: [ + 'Set on-graph logging (notifier) level', + 'This should ONLY be set via Plotly.setPlotConfig', + 'Available levels:', + '0: no on-graph logs', + '1: warnings and errors, but not informational messages', + '2: verbose logs' + ].join(' ') + }, + queueLength: { valType: 'integer', min: 0, diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index cbcdfb2ec18..d855b331ded 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -1671,8 +1671,9 @@ describe('Test lib.js:', function() { }); describe('loggers', function() { - var stashConsole, - stashLogLevel; + var stashConsole; + var stashLogLevel; + var stashOnGraphLogLevel; function consoleFn(name, hasApply, messages) { var out = function() { @@ -1703,11 +1704,13 @@ describe('Test lib.js:', function() { beforeEach(function() { stashConsole = window.console; stashLogLevel = config.logging; + stashOnGraphLogLevel = config.onGraphLogging; }); afterEach(function() { window.console = stashConsole; config.logging = stashLogLevel; + config.onGraphLogging = stashOnGraphLogLevel; }); it('emits one console message if apply is available', function() { @@ -1807,6 +1810,50 @@ describe('Test lib.js:', function() { ['log', [{a: 1, b: 2}]] ]); }); + + describe('should log message in notifier div in accordance onGraphLogging config option', function() { + var query = '.notifier-note'; + + beforeEach(function(done) { + d3.selectAll(query).each(function() { + d3.select(this).select('button').node().click(); + }); + setTimeout(done, 1000); + }); + + function _run(exp) { + config.logging = 0; + + Lib.log('log'); + Lib.warn('warn'); + Lib.error('error!'); + + var notes = d3.selectAll(query); + + expect(notes.size()).toBe(exp.length, '# of notifier notes'); + + var actual = []; + notes.each(function() { + actual.push(d3.select(this).select('p').text()); + }); + expect(actual).toEqual(exp); + } + + it('with level 2', function() { + config.onGraphLogging = 2; + _run(['log', 'warn', 'error!']); + }); + + it('with level 1', function() { + config.onGraphLogging = 1; + _run(['warn', 'error!']); + }); + + it('with level 0', function() { + config.onGraphLogging = 0; + _run([]); + }); + }); }); describe('keyedContainer', function() { From e4245a69d6c90a29f21cac236209ccdccad20130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 30 Dec 2019 15:46:53 -0500 Subject: [PATCH 3/6] call Lib.error before throwing error for no-token/missing-token issues ... so that the error message can show up on the graph when the onGraphLogging config option is turned on. --- src/plots/mapbox/index.js | 1 + test/jasmine/tests/mapbox_test.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plots/mapbox/index.js b/src/plots/mapbox/index.js index 9efcafca2a0..c9e93b61d6f 100644 --- a/src/plots/mapbox/index.js +++ b/src/plots/mapbox/index.js @@ -250,6 +250,7 @@ function findAccessToken(gd, mapboxIds) { var msg = hasOneSetMapboxStyle ? constants.noAccessTokenErrorMsg : constants.missingStyleErrorMsg; + Lib.error(msg); throw new Error(msg); } diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 75d78fc738b..ebe795b20c3 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -295,7 +295,7 @@ describe('mapbox credentials', function() { }]); }).toThrow(new Error(constants.missingStyleErrorMsg)); - expect(Lib.error).toHaveBeenCalledTimes(0); + expect(Lib.error).toHaveBeenCalledWith(constants.missingStyleErrorMsg); }, LONG_TIMEOUT_INTERVAL); it('@gl should throw error when setting a Mapbox style w/o a registered token', function() { From b0cf1d099d0a8547201a22306740243b7b52f9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 6 Jan 2020 13:52:51 -0500 Subject: [PATCH 4/6] reduce transition duration for 'stick' notifier notes --- src/lib/notifier.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/notifier.js b/src/lib/notifier.js index 5a7689eea21..89b63ede164 100644 --- a/src/lib/notifier.js +++ b/src/lib/notifier.js @@ -72,7 +72,7 @@ module.exports = function(text, displayLength) { if(displayLength === 'stick') { note.transition() - .duration(700) + .duration(350) .style('opacity', 1); } else { note.transition() From 75a741442a70a3e5c527e4cb7ca2c8a634b48789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 6 Jan 2020 14:45:04 -0500 Subject: [PATCH 5/6] add treemap_with-without_values_template mock to flaky list --- test/image/compare_pixels_test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/image/compare_pixels_test.js b/test/image/compare_pixels_test.js index 2ada885b019..f8a1ca22f91 100644 --- a/test/image/compare_pixels_test.js +++ b/test/image/compare_pixels_test.js @@ -102,6 +102,7 @@ if(allMock || argv.filter) { var FLAKY_LIST = [ 'treemap_textposition', + 'treemap_with-without_values_template', 'trace_metatext', 'gl3d_directions-streamtube1' ]; From 2bff8965a78309259ca2e2b7095a077e9f931970 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 6 Jan 2020 16:06:05 -0500 Subject: [PATCH 6/6] sub onGraphLogging with notifyOnLogging --- src/lib/loggers.js | 6 +++--- src/plot_api/plot_config.js | 2 +- test/jasmine/tests/lib_test.js | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/lib/loggers.js b/src/lib/loggers.js index 56a1c863a8f..e6bb04224c9 100644 --- a/src/lib/loggers.js +++ b/src/lib/loggers.js @@ -33,7 +33,7 @@ loggers.log = function() { apply(console.trace || console.log, messages); } - if(dfltConfig.onGraphLogging > 1) { + if(dfltConfig.notifyOnLogging > 1) { var lines = []; for(i = 0; i < arguments.length; i++) { lines.push(arguments[i]); @@ -53,7 +53,7 @@ loggers.warn = function() { apply(console.trace || console.log, messages); } - if(dfltConfig.onGraphLogging > 0) { + if(dfltConfig.notifyOnLogging > 0) { var lines = []; for(i = 0; i < arguments.length; i++) { lines.push(arguments[i]); @@ -73,7 +73,7 @@ loggers.error = function() { apply(console.error, messages); } - if(dfltConfig.onGraphLogging > 0) { + if(dfltConfig.notifyOnLogging > 0) { var lines = []; for(i = 0; i < arguments.length; i++) { lines.push(arguments[i]); diff --git a/src/plot_api/plot_config.js b/src/plot_api/plot_config.js index 309e0c708ce..87c3d8e4ad7 100644 --- a/src/plot_api/plot_config.js +++ b/src/plot_api/plot_config.js @@ -390,7 +390,7 @@ var configAttributes = { ].join(' ') }, - onGraphLogging: { + notifyOnLogging: { valType: 'integer', min: 0, max: 2, diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index d855b331ded..34d5d2c43a6 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -1704,13 +1704,13 @@ describe('Test lib.js:', function() { beforeEach(function() { stashConsole = window.console; stashLogLevel = config.logging; - stashOnGraphLogLevel = config.onGraphLogging; + stashOnGraphLogLevel = config.notifyOnLogging; }); afterEach(function() { window.console = stashConsole; config.logging = stashLogLevel; - config.onGraphLogging = stashOnGraphLogLevel; + config.notifyOnLogging = stashOnGraphLogLevel; }); it('emits one console message if apply is available', function() { @@ -1811,7 +1811,7 @@ describe('Test lib.js:', function() { ]); }); - describe('should log message in notifier div in accordance onGraphLogging config option', function() { + describe('should log message in notifier div in accordance notifyOnLogging config option', function() { var query = '.notifier-note'; beforeEach(function(done) { @@ -1840,17 +1840,17 @@ describe('Test lib.js:', function() { } it('with level 2', function() { - config.onGraphLogging = 2; + config.notifyOnLogging = 2; _run(['log', 'warn', 'error!']); }); it('with level 1', function() { - config.onGraphLogging = 1; + config.notifyOnLogging = 1; _run(['warn', 'error!']); }); it('with level 0', function() { - config.onGraphLogging = 0; + config.notifyOnLogging = 0; _run([]); }); });