-
Notifications
You must be signed in to change notification settings - Fork 241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added possibility to share multiple calendars #865
Conversation
js/app/controllers/calcontroller.js
Outdated
} | ||
|
||
$scope.integration = function () { |
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.
Why did you move this here?
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.
Moved here, because otherwise the embed code will only contain one of the public sharing tokens.
js/app/controllers/calcontroller.js
Outdated
var tokens = constants.publicSharingToken.split("."); | ||
var finished = 0; | ||
tokens.forEach(function(token){ | ||
$scope.calendarsPromise = CalendarService.getPublicCalendar(token).then(function(calendar) { |
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.
Please ident with tabs instead of spaces
js/app/controllers/calcontroller.js
Outdated
$scope.$apply(); | ||
} | ||
}).catch((reason) => { | ||
angular.element('#header-right').css('display', 'none'); |
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.
This error handling needs to be adapted.
You don't want to show the empty-container error page if one out of ten calendars fail.
You would want some kind of Notification though. You can use OC.Notification.show()
for that.
js/app/controllers/calcontroller.js
Outdated
} | ||
|
||
$scope.integration = function () { | ||
return '<iframe width="400" height="215" src="' + $scope.$parent.root + 'embed/' + constants.publicSharingToken + '"></iframe>'; |
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.
Not sure if angular handles escaping here.
What if you pass html as a parameter for constants.publicSharingToken
?
This might result in an html injection.
Can you please check if its properly escaped?
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.
Will check
I messed up commits, which automatically closed this PR. |
This PR implements a function requested in #708
You can pass multiple share tokens split by a . to embed multiple calendars.
All calendars are shown in the default color at the moment. This may be impoved.