Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 856888 - Support links was added in calendar's notes #33545

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apps/calendar/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
<script defer src="/shared/elements/gaia_checkbox/script.js"></script>
<script defer src="/shared/elements/gaia_switch/script.js"></script>
<script defer src="shared/elements/config.js"></script>

<script defer src="/shared/js/links/activity_picker.js"></script>
<script defer src="shared/js/links/link_helper.js"></script>
<script defer src="js/common/link_action_handler.js"></script>

<link rel="stylesheet" type="text/css" href="app://theme.gaiamobile.org/shared/elements/gaia-theme/gaia-theme.css" />
<script defer src="shared/elements/gaia-header/dist/gaia-header.js"></script>
<link rel="stylesheet" type="text/css" href="app://theme.gaiamobile.org/shared/elements/gaia-theme/style.css" />
Expand Down
29 changes: 29 additions & 0 deletions apps/calendar/js/common/link_action_handler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* global ActivityPicker */

define(function(require, exports, module) {
'use strict';
Copy link
Contributor

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

/*
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
);

};

});
14 changes: 13 additions & 1 deletion apps/calendar/js/views/view_event.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* global LinkHelper */

define(function(require, exports, module) {
'use strict';

Expand All @@ -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');

Expand All @@ -27,6 +30,13 @@ ViewEvent.prototype = {

_initEvents: function() {
EventBase.prototype._initEvents.apply(this, arguments);
this.getEl('description').addEventListener('click', this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to listen in onactive and then remove the listener in oninactive

},

handleEvent: function(e) {
if (e.type === 'click') {
linkActionHandler.onClick(e);
}
},

/**
Expand Down Expand Up @@ -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() {
Expand Down
5 changes: 5 additions & 0 deletions apps/calendar/style/event_view.css
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,8 @@
#event-view .current-calendar .content {
margin-inline-start: 1.2rem;
}

#event-view .description a {
color: #00F;
text-decoration: underline;
}
4 changes: 2 additions & 2 deletions apps/sms/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably needs an sms reviewer also?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
But would you do me a favor and invite some competent reviewer for the sms app to review my code from this side?

Other your comments are fixed.

Thanks,

Copy link
Contributor

Choose a reason for hiding this comment

The 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>
Expand Down
4 changes: 2 additions & 2 deletions apps/sms/views/conversation/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
<script defer src="/views/shared/js/wbmp.js"></script>
<script defer src="/views/shared/js/smil.js"></script>
<script defer src="/views/shared/js/shared_components.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="/shared/js/image_utils.js"></script>
<script defer src="/views/conversation/js/attachment.js"></script>
<script defer src="/views/conversation/js/attachment_renderer.js"></script>
Expand All @@ -111,7 +111,7 @@
<script defer src="/views/shared/js/dialog.js"></script>
<script defer src="/views/shared/js/error_dialog.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>
<script defer src="/views/conversation/js/link_action_handler.js"></script>
<script defer src="/views/shared/js/contact_renderer.js"></script>
<script defer src="/views/shared/js/localization_helper.js"></script>
Expand Down
2 changes: 1 addition & 1 deletion apps/sms/views/conversation/js/startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
'/views/shared/js/error_dialog.js',
'/views/conversation/js/link_action_handler.js',
'/views/shared/js/contact_renderer.js',
'/views/shared/js/activity_picker.js',
'/shared/js/links/activity_picker.js',
'/views/conversation/js/information.js',
'/views/shared/js/localization_helper.js'
];
Expand Down
8 changes: 4 additions & 4 deletions apps/sms/views/conversation/test/unit/link_helper_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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"' +
Copy link
Contributor

Choose a reason for hiding this comment

The 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);
});
Expand Down
2 changes: 1 addition & 1 deletion apps/sms/views/inbox/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
<script defer src="/shared/js/option_menu.js"></script>
<script defer src="/views/shared/js/dialog.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>
-->
</head>

Expand Down
2 changes: 1 addition & 1 deletion apps/sms/views/inbox/js/startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
'/views/shared/js/waiting_screen.js',
'/views/shared/js/dialog.js',
'/views/shared/js/error_dialog.js',
'/views/shared/js/activity_picker.js'
'/shared/js/links/activity_picker.js'
];

function initLazyDependencies() {
Expand Down
4 changes: 2 additions & 2 deletions apps/sms/views/shared/js/startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ var Startup = exports.Startup = {
'/views/shared/js/errors.js',
'/views/shared/js/dialog.js',
'/views/shared/js/error_dialog.js',
'/views/conversation/js/link_helper.js',
'/shared/js/links/link_helper.js',
'/views/conversation/js/link_action_handler.js',
'/views/shared/js/contact_renderer.js',
'/views/shared/js/activity_picker.js',
'/shared/js/links/activity_picker.js',
'/views/conversation/js/information.js',
'/views/shared/js/shared_components.js',
'/views/shared/js/task_runner.js',
Expand Down
4 changes: 1 addition & 3 deletions apps/sms/views/shared/test/unit/activity_picker_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@

require('/shared/test/unit/mocks/mock_l20n.js');

require('/views/shared/js/activity_picker.js');
require('/views/shared/js/utils.js');
require('/shared/js/links/activity_picker.js');

require('/views/shared/test/unit/mock_moz_activity.js');
require('/views/shared/test/unit/mock_utils.js');


var mocksHelperAP = new MocksHelper([
'MozActivity',
'Utils'
]).init();

suite('ActivityPicker', function() {
Expand Down
2 changes: 1 addition & 1 deletion apps/sms/views/shared/test/unit/sms_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ require('/views/shared/test/unit/thread_list_mockup.js');
require('/views/shared/js/utils.js');
require('/views/shared/js/selection_handler.js');
require('/views/shared/js/navigation.js');
require('/views/conversation/js/link_helper.js');
require('/shared/js/links/link_helper.js');
require('/services/js/drafts.js');
require('/views/shared/js/contacts.js');
require('/views/conversation/js/subject_composer.js');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
/*global Utils */

(function() {
'use strict';
/*
Expand Down Expand Up @@ -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, '');
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
Expand All @@ -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);

Expand Down Expand Up @@ -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>';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: was this needed ?

}
},

Expand Down