-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Bug 856888 - Support links was added in calendar's notes #33545
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* global ActivityPicker */ | ||
|
||
define(function(require, exports, module) { | ||
'use strict'; | ||
/* | ||
Centralized event handling for various | ||
data-actions url, email, phone in a message | ||
*/ | ||
|
||
module.exports.onClick = function lah_onClick(event) { | ||
event.preventDefault(); | ||
event.stopPropagation(); | ||
|
||
var dataset = event.target.dataset; | ||
var action = dataset.action; | ||
|
||
if (!action) { | ||
return; | ||
} | ||
|
||
var type = action.replace('-link', ''); | ||
|
||
ActivityPicker[type]( | ||
dataset[type], this.reset, this.reset | ||
); | ||
|
||
}; | ||
|
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
/* global LinkHelper */ | ||
|
||
define(function(require, exports, module) { | ||
'use strict'; | ||
|
||
|
@@ -6,6 +8,7 @@ var EventBase = require('./event_base'); | |
var alarmTemplate = require('templates/alarm'); | ||
var localCalendarId = require('common/constants').localCalendarId; | ||
var router = require('router'); | ||
var linkActionHandler = require('common/link_action_handler'); | ||
|
||
require('dom!event-view'); | ||
|
||
|
@@ -27,6 +30,13 @@ ViewEvent.prototype = { | |
|
||
_initEvents: function() { | ||
EventBase.prototype._initEvents.apply(this, arguments); | ||
this.getEl('description').addEventListener('click', this); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you want to listen in |
||
}, | ||
|
||
handleEvent: function(e) { | ||
if (e.type === 'click') { | ||
linkActionHandler.onClick(e); | ||
} | ||
}, | ||
|
||
/** | ||
|
@@ -126,7 +136,9 @@ ViewEvent.prototype = { | |
} | ||
|
||
this.setContent('alarms', alarmContent, 'innerHTML'); | ||
this.setContent('description', model.description); | ||
var description = LinkHelper.searchAndLinkClickableData(model.description); | ||
|
||
this.setContent('description', description, 'innerHTML'); | ||
}, | ||
|
||
oninactive: function() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -115,10 +115,10 @@ | |
<script defer src="views/conversation/js/subject_composer.js"></script> | ||
<script defer src="views/conversation/js/compose.js"></script> | ||
<script defer src="views/shared/js/waiting_screen.js"></script> | ||
<script defer src="views/shared/js/activity_picker.js"></script> | ||
<script defer src="shared/js/links/activity_picker.js"></script> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably needs an sms reviewer also? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gaye , as you could see I moved activity_picker.js from views/shared/js to shared/js/links. And I have used this module in sms and calendar apps. I tried to replace all links that necessary in sms app and after testing the app, as I could notice, it works as it worked. Other your comments are fixed. Thanks, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @julienw ping! |
||
<script defer src="views/shared/js/wbmp.js"></script> | ||
<script defer src="views/shared/js/smil.js"></script> | ||
<script defer src="views/conversation/js/link_helper.js"></script> | ||
<script defer src="shared/js/links/link_helper.js"></script> | ||
<script defer src="views/conversation/js/link_action_handler.js"></script> | ||
<script defer src="views/shared/js/notify.js"></script> | ||
<script defer src="lib/bridge/client.js"></script> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,8 @@ | |
*/ | ||
'use strict'; | ||
|
||
require('/views/conversation/js/link_helper.js'); | ||
require('/shared/js/links/link_helper.js'); | ||
|
||
require('/views/shared/js/utils.js'); | ||
require('/views/shared/test/unit/mock_utils.js'); | ||
|
||
suite('link_helper_test.js', function() { | ||
|
@@ -366,8 +365,9 @@ suite('link_helper_test.js', function() { | |
'<a data-url="http://stackoverflow.com/q/12882966/" ' + | ||
'data-action="url-link" >http://stackoverflow.com/q/12882966/</a>' + | ||
' and call me at ' + | ||
'<a dir="ltr" data-dial="+18155551212" data-action="dial-link">' + | ||
'+18155551212</a> or (e-mail <a data-email="user@hostname.tld"' + | ||
'<a dir="ltr" data-dial="+18155551212" ' + | ||
'data-action="dial-link">+18155551212</a> or (e-mail ' + | ||
'<a data-email="user@hostname.tld"' + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not that it's so important, but why this change ? |
||
' data-action="email-link">user@hostname.tld</a>)'; | ||
assert.equal(LinkHelper.searchAndLinkClickableData(test), expected); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,3 @@ | ||
/*global Utils */ | ||
|
||
(function() { | ||
'use strict'; | ||
/* | ||
|
@@ -59,6 +57,12 @@ function checkDomain(domain) { | |
|
||
// defines things that can match right before to be a "safe" link | ||
var safeStart = /[\s,:;\(>\u0080-\uFFFF]/; | ||
var Utils = { | ||
rnondialablechars: /[^,#+\*\d]/g, | ||
removeNonDialables: function removeNonDialables(input){ | ||
return input.replace(this.rnondialablechars, ''); | ||
} | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: maybe this could just be a normal function instead of a method ? |
||
|
||
const MINIMUM_DIGITS_IN_PHONE_NUMBER = 5; | ||
const LEADING_DIGIT_BREAKPOINT = 7; | ||
|
@@ -84,6 +88,7 @@ var LINK_TYPES = { | |
'(?:\\d[\\d \\t.()-]{0,12}\\d)' + // <digit><digit|sddp>*<digit> | ||
'\\b' // must end on a word boundary | ||
].join(''), 'g'), | ||
|
||
matchFilter: function phoneMatchFilter(phone, link) { | ||
var onlyDigits = Utils.removeNonDialables(phone); | ||
|
||
|
@@ -161,8 +166,8 @@ var LINK_TYPES = { | |
if (!linkSpec.match[1]) { | ||
href = 'http://' + href; | ||
} | ||
return '<a data-url="' + href + '" data-action="url-link" >' + url + | ||
'</a>'; | ||
return '<a data-url="' + href + '" data-action="url-link" >' + | ||
url + '</a>'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: was this needed ? |
||
} | ||
}, | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Move code inside module 2 spaces left