From eeafb1187e54f7c63440a254d6e6077f255e4958 Mon Sep 17 00:00:00 2001 From: Nick Wittwer Date: Wed, 3 Oct 2018 17:07:47 +0200 Subject: [PATCH 1/7] Added static test for input urls --- test/static/url-input.html | 118 +++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 test/static/url-input.html diff --git a/test/static/url-input.html b/test/static/url-input.html new file mode 100644 index 00000000..a06da1b0 --- /dev/null +++ b/test/static/url-input.html @@ -0,0 +1,118 @@ + + + + + + + URL Input Test + + + + + + + + + + + \ No newline at end of file From 38bdba784f6c3588d8e4ec2536060c47c214e15e Mon Sep 17 00:00:00 2001 From: Nick Wittwer Date: Wed, 3 Oct 2018 18:00:04 +0200 Subject: [PATCH 2/7] Added new script to toolbar init --- src/js/features/toolbar/toolbar.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/js/features/toolbar/toolbar.js b/src/js/features/toolbar/toolbar.js index 614819c4..dab15c25 100644 --- a/src/js/features/toolbar/toolbar.js +++ b/src/js/features/toolbar/toolbar.js @@ -3,16 +3,26 @@ app.toolbar = { init: function () { this.firstLoad(); app.toolbar.zoomControls.init(); + app.toolbar.recentURLs.init(); + app.toolbar.smartURL.init(); }, firstLoad: function () { - app.toolbar.recentURLs.init(); - // Bind the "Enter" key => load URL in artboardInnerFrame $("#toolbar__url").on('keypress', function (e) { if (e.which == 13) { e.preventDefault(); + + // As long as the SmartURL is able to be set... + var initialURL = $("#toolbar__url").val(); + if ( app.toolbar.smartURL.make(initialURL) !== false ) { + // Update the URL + var result = app.toolbar.smartURL.make(initialURL); + $("#toolbar__url").val(result); + } + app.toolbar.updateURL(); + } }); }, From e38b72441df4e18e052c129682d18275b852d5de Mon Sep 17 00:00:00 2001 From: Nick Wittwer Date: Wed, 3 Oct 2018 18:00:31 +0200 Subject: [PATCH 3/7] Added app.toolbar.smartURL.make() function and test case --- src/js/features/toolbar/toolbar-smart-urls.js | 71 +++++++++++++++++++ test/static/url-input.html | 32 ++++++--- 2 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 src/js/features/toolbar/toolbar-smart-urls.js diff --git a/src/js/features/toolbar/toolbar-smart-urls.js b/src/js/features/toolbar/toolbar-smart-urls.js new file mode 100644 index 00000000..fe8a3113 --- /dev/null +++ b/src/js/features/toolbar/toolbar-smart-urls.js @@ -0,0 +1,71 @@ +app.toolbar.smartURL = { + init: function () {}, + + // Makes a smart URL + // Returns a new URL or false if there was an issue + make: function (s) { + let url = s.toLowerCase(); + let hasHttpPrefix = false; + let hasDot = false; + let hasLocalhost = false; + let failed = false; + + // Step 1: Does it have http:// or https:// ? + // The "?" in the Regex accepts http or https + if (new RegExp(/https?/).test(url) == true) { + hasHttpPrefix = true; + } + + // Step 2: Does it have .* (i.e. ".com") ? + if (url.includes('.') == true) { + hasDot = true; + } + + // Step 3: Does it include "localhost"? + if (url.includes('localhost') == true) { + hasLocalhost = true; + } + + // Logic + if (hasHttpPrefix && hasDot) { + // Perfect format: + // http[s]://example.com + } else if (hasHttpPrefix == false && hasDot == true && hasLocalhost == false) { + // Case: example.com + // Check if URL starts with anything besides a letter or digit + if (new RegExp(/^[0-9a-z]/).test(url) == false) { + failed = true; + } else { + // The URL can be prepended by http:// + url = "http://" + url; + } + } else if (hasLocalhost == true) { + if (!hasHttpPrefix) { + // Case: "localhost:8000" + url = "http://" + url; + } + } else if (hasHttpPrefix == true && hasDot == false && hasLocalhost == false) { + // no pass, there's no ".com" or similar ending + failed = true; + } else { + // Empty string or unknown error + failed = true; + } + + // Fail cases where there's no http/http and no top-level domain + if (hasHttpPrefix == false && hasDot == false && hasLocalhost == false) { + failed = true; + } + + if (failed === true) { + alert(url + " is not a valid URL."); + } + + // Add handler below + if (failed === true) { + return false; + } else { + return url; + } + } +} \ No newline at end of file diff --git a/test/static/url-input.html b/test/static/url-input.html index a06da1b0..9ce911a2 100644 --- a/test/static/url-input.html +++ b/test/static/url-input.html @@ -40,6 +40,8 @@ "123example.com", // Missing http, should strip the inital symbols "123.example.com", // Missing http, should strip the inital symbols "123-example.com", // Missing http, should strip the inital symbols + "localhost:8000", // Missing http, should strip the inital symbols + "http://localhost:8000", // Missing http, should strip the inital symbols // The following should fail: "https://example", // Full URL with file path @@ -52,11 +54,10 @@ function testURL(s) { let url = s.toLowerCase(); - - var hasHttpPrefix = false; - var hasDot = false; - - var failed = false; + let hasHttpPrefix = false; + let hasDot = false; + let hasLocalhost = false; + let failed = false; // Step 1: Does it have http:// or https:// ? // The "?" in the Regex accepts http or https @@ -69,11 +70,17 @@ hasDot = true; } + // Step 3: Does it include "localhost"? + if (url.includes('localhost') == true) { + hasLocalhost = true; + } + // Logic if (hasHttpPrefix && hasDot) { - // It's perfect :) - // Format: http[s]://example.com - } else if (hasHttpPrefix == false && hasDot == true) { + // Perfect format: + // http[s]://example.com + } else if (hasHttpPrefix == false && hasDot == true && hasLocalhost == false) { + // Case: example.com // Check if URL starts with anything besides a letter or digit if (new RegExp(/^[0-9a-z]/).test(url) == false) { failed = true; @@ -81,7 +88,12 @@ // The URL can be prepended by http:// url = "http://" + url; } - } else if (hasHttpPrefix == true && hasDot == false) { + } else if (hasLocalhost == true) { + if ( !hasHttpPrefix ) { + // Case: "localhost:8000" + url = "http://" + url; + } + } else if (hasHttpPrefix == true && hasDot == false && hasLocalhost == false) { // no pass, there's no ".com" or similar ending failed = true; } else { @@ -91,7 +103,7 @@ } // Fail cases where there's no http/http and no top-level domain - if (hasHttpPrefix == false && hasDot == false) { + if (hasHttpPrefix == false && hasDot == false && hasLocalhost == false) { failed = true; } From b739714695f84a82e8d06f85286d2124d83e2643 Mon Sep 17 00:00:00 2001 From: Nick Wittwer Date: Wed, 3 Oct 2018 18:15:51 +0200 Subject: [PATCH 4/7] Removed redundancy of one if statement --- src/js/features/toolbar/toolbar-smart-urls.js | 5 +---- test/static/url-input.html | 7 ++----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/js/features/toolbar/toolbar-smart-urls.js b/src/js/features/toolbar/toolbar-smart-urls.js index fe8a3113..729277c1 100644 --- a/src/js/features/toolbar/toolbar-smart-urls.js +++ b/src/js/features/toolbar/toolbar-smart-urls.js @@ -57,12 +57,9 @@ app.toolbar.smartURL = { failed = true; } - if (failed === true) { - alert(url + " is not a valid URL."); - } - // Add handler below if (failed === true) { + alert(url + " is not a valid URL."); return false; } else { return url; diff --git a/test/static/url-input.html b/test/static/url-input.html index 9ce911a2..7de2f611 100644 --- a/test/static/url-input.html +++ b/test/static/url-input.html @@ -106,14 +106,11 @@ if (hasHttpPrefix == false && hasDot == false && hasLocalhost == false) { failed = true; } - - if (failed === true) { - console.log(url + " is not a valid URL."); - } - + // Add handler below // For demo purposes, it highlights the failed tests in red if (failed === true) { + console.log(url + " is not a valid URL."); $("body").append(`
${s} => ${url} `); } else { $("body").append(`
${s} => ${url} `); From 3e789f232cb3512dd8391add1e7295baa59640f0 Mon Sep 17 00:00:00 2001 From: Nick Wittwer Date: Wed, 3 Oct 2018 22:46:53 +0200 Subject: [PATCH 5/7] Updated toolbar.js so that it only saves to recent searches when the URL is valid --- src/js/features/toolbar/toolbar.js | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/js/features/toolbar/toolbar.js b/src/js/features/toolbar/toolbar.js index dab15c25..4519d543 100644 --- a/src/js/features/toolbar/toolbar.js +++ b/src/js/features/toolbar/toolbar.js @@ -13,15 +13,23 @@ app.toolbar = { if (e.which == 13) { e.preventDefault(); - // As long as the SmartURL is able to be set... - var initialURL = $("#toolbar__url").val(); - if ( app.toolbar.smartURL.make(initialURL) !== false ) { - // Update the URL - var result = app.toolbar.smartURL.make(initialURL); + // Try to turn the URL into a "smart" URL + // This improves the UX when someone types in something like "localhost:8000" + // it will automatically prepend "http://" for them or deal with invalid URLs + var url = $("#toolbar__url").val(); + if ( app.toolbar.smartURL.make(url) !== false ) { + var result = app.toolbar.smartURL.make(url); $("#toolbar__url").val(result); + + // Save the URL to LocalStorage RecentURLs + app.toolbar.recentURLs.add(e); + + // Now update the URL + app.toolbar.updateURL(); + } else { + // Error, not a valid + alert(`${url} is not a valid URL`); } - - app.toolbar.updateURL(); } }); From 0440edaa209601caa6bd91c70e3320f8b01eeaa8 Mon Sep 17 00:00:00 2001 From: Nick Wittwer Date: Wed, 3 Oct 2018 22:47:46 +0200 Subject: [PATCH 6/7] Removed .on() event for recent urls; this is handled inside of toolbar.js now --- src/js/features/toolbar/toolbar-recent-urls.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/js/features/toolbar/toolbar-recent-urls.js b/src/js/features/toolbar/toolbar-recent-urls.js index 2b3ff882..a5ee520d 100644 --- a/src/js/features/toolbar/toolbar-recent-urls.js +++ b/src/js/features/toolbar/toolbar-recent-urls.js @@ -16,21 +16,11 @@ app.toolbar.recentURLs = { app.toolbar.recentURLs.show(); // Listen for clicks on the dropdown items - $(".toolbar__url-li").on('click', function(e) { - $("#toolbar__url").val( $(e.target).text() ); + $(".toolbar__url-li").on('click', function (e) { + $("#toolbar__url").val($(e.target).text()); app.toolbar.updateURL(); }); - // Add to LocalStorage on submit - $("#toolbar__url").on('submit keypress', function (e) { - // On enter key press - if (e.which == 13) { - app.toolbar.recentURLs.add(e); - } else if (e.type == "submit") { - app.toolbar.recentURLs.add(e); - } - }); - // When clicking the search, show recent sites $("#toolbar__url").on('click blur', function (e) { if (e.type == "click") { From 2d07f94fe4df36a90d7deb62d54c07082b8e659e Mon Sep 17 00:00:00 2001 From: Nick Wittwer Date: Wed, 3 Oct 2018 22:48:41 +0200 Subject: [PATCH 7/7] Improved+extended tests, minor logic refactoring --- src/js/features/toolbar/toolbar-smart-urls.js | 19 +-- test/static/url-input.html | 114 ++++++++++++------ 2 files changed, 85 insertions(+), 48 deletions(-) diff --git a/src/js/features/toolbar/toolbar-smart-urls.js b/src/js/features/toolbar/toolbar-smart-urls.js index 729277c1..fc7440b1 100644 --- a/src/js/features/toolbar/toolbar-smart-urls.js +++ b/src/js/features/toolbar/toolbar-smart-urls.js @@ -3,13 +3,18 @@ app.toolbar.smartURL = { // Makes a smart URL // Returns a new URL or false if there was an issue - make: function (s) { - let url = s.toLowerCase(); + make: function (url) { + if (typeof url !== 'string') { + throw new TypeError(`Expected \`url\` to be of type \`string\`, got \`${typeof url}\``); + } + let hasHttpPrefix = false; let hasDot = false; let hasLocalhost = false; let failed = false; + url = url.toLowerCase(); + // Step 1: Does it have http:// or https:// ? // The "?" in the Regex accepts http or https if (new RegExp(/https?/).test(url) == true) { @@ -27,7 +32,7 @@ app.toolbar.smartURL = { } // Logic - if (hasHttpPrefix && hasDot) { + if (hasHttpPrefix && hasDot || hasLocalhost) { // Perfect format: // http[s]://example.com } else if (hasHttpPrefix == false && hasDot == true && hasLocalhost == false) { @@ -39,11 +44,6 @@ app.toolbar.smartURL = { // The URL can be prepended by http:// url = "http://" + url; } - } else if (hasLocalhost == true) { - if (!hasHttpPrefix) { - // Case: "localhost:8000" - url = "http://" + url; - } } else if (hasHttpPrefix == true && hasDot == false && hasLocalhost == false) { // no pass, there's no ".com" or similar ending failed = true; @@ -59,7 +59,8 @@ app.toolbar.smartURL = { // Add handler below if (failed === true) { - alert(url + " is not a valid URL."); + // Handle errors here + // Example: alert(url + " is not a valid URL."); return false; } else { return url; diff --git a/test/static/url-input.html b/test/static/url-input.html index 7de2f611..03d8a062 100644 --- a/test/static/url-input.html +++ b/test/static/url-input.html @@ -10,9 +10,15 @@ body, html { font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol"; - line-height: 2; + line-height: 1.5; margin: 0; - padding: 0; + padding: 2rem 4rem; + } + .item { + margin-bottom: 2rem; + } + .label { + display: block; } .success { color: green; @@ -27,38 +33,45 @@