Skip to content

Commit

Permalink
Merge pull request #80 from nwittwer/issue-73
Browse files Browse the repository at this point in the history
Issue 73 - "Smart URLs"
  • Loading branch information
nwittwer authored Oct 3, 2018
2 parents 8ac7518 + 2d07f94 commit 01b1465
Show file tree
Hide file tree
Showing 4 changed files with 255 additions and 15 deletions.
14 changes: 2 additions & 12 deletions src/js/features/toolbar/toolbar-recent-urls.js
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
69 changes: 69 additions & 0 deletions src/js/features/toolbar/toolbar-smart-urls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
app.toolbar.smartURL = {
init: function () {},

// Makes a smart URL
// Returns a new URL or false if there was an issue
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) {
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 || hasLocalhost) {

This comment has been minimized.

Copy link
@nwittwer

nwittwer Oct 4, 2018

Author Collaborator

The hasLocalhost flag here is a problem—it should be transforming hasLocalhost URLs into the proper format, not just allowing them to pass

// 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 (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;
}

// Add handler below
if (failed === true) {
// Handle errors here
// Example: alert(url + " is not a valid URL.");
return false;
} else {
return url;
}
}
}
24 changes: 21 additions & 3 deletions src/js/features/toolbar/toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,34 @@ 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();
app.toolbar.updateURL();

// 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`);
}

}
});
},
Expand Down
163 changes: 163 additions & 0 deletions test/static/url-input.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
<!DOCTYPE html>
<html>

<head>
<meta charset="utf-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<title>URL Input Test</title>
<meta name="viewport" content="width=device-width, initial-scale=1">
<style>
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: 1.5;
margin: 0;
padding: 2rem 4rem;
}
.item {
margin-bottom: 2rem;
}
.label {
display: block;
}
.success {
color: green;
}
.failed {
color: red;
}
</style>
</head>

<body>

<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<script type="text/javascript">
var cases = {
"Full URL": "http://example.com",
"Full URL with uppercase": "HTTP://EXAMPLE.COM",
"Full URL, https": "https://example.com",
"Full URL with file path": "https://example.com/file/path.html",
"Full URL + file path + dash": "https://example.com/file/path-dash.html",
"Full URL + file path + underscore": "https://example.com/file/path_dash.html",
"Missing http:// + .com": "example.com",
"Missing http:// + non-.com domain": "example.co.uk",
"Missing http:// + non-.com domain 2": "example.io",
"Number + string": "123example.com",
"Subdomain": "123.example.com",
"Number + dash + .com": "123-example.com",
"Missing HTTP": "localhost:8000",
"Http + Localhost": "http://localhost:8000",
"Http + local URL": "http: //nw.local:5757",
"Http + number + port": "http://192.168.0.3:5757/",

// The following should fail:
"Missing .com or similar": "https://example",
"Misc. symbols": "://example.com",
"Misc. symbols + file path + dash": "://example.com/file/path-dash.html",
"Misc. symbol before": "/example.com", //
"Missing http:// and missing .com": "example",
"Empty string": "" // Empty
};

function testURL(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) {
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 || hasLocalhost) {
// 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 (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;
}

// Add handler below
if (failed === true) {
// Handle errors here
// Example: alert(url + " is not a valid URL.");
return false;
} else {
return url;
}
}

// Iterate over each test case
for (let i = 0, len = cases.length; i < len; i++) {
var test = testURL(cases[i]);

if (test !== false) {
$("body").append(`<br> ${cases[i]} => <span class='success'> ${test} </span>`);
} else {
$("body").append(`<br> ${cases[i]} => <span class='failed'>false</span>`);
}
}

for (const [key, value] of Object.entries(cases)) {
console.log(key, value);

var test = testURL(cases[key]);

if (test !== false) {
$("body").append(
`<div class="item">
<div class="label">${key}:</div>
${value} => <span class='success'>${test}</span>
</div>`
);
} else {
$("body").append(
`<div class="item">
<div class="label">${key}:</div>
${value} => <span class='failed'>${test}</span>
</div>`
);
}

}
</script>
</body>

</html>

0 comments on commit 01b1465

Please sign in to comment.