Skip to content
This repository was archived by the owner on May 4, 2022. It is now read-only.

Add from feature button#692

Merged
Doug-Reed merged 6 commits intomasterfrom
addFromFeatureButton
Sep 15, 2017
Merged

Add from feature button#692
Doug-Reed merged 6 commits intomasterfrom
addFromFeatureButton

Conversation

@Doug-Reed
Copy link
Contributor

Adds functionality to support adding a widget to the homepage from a mascot announcement


Contributor License Agreement adherence:


$rootScope.addPortletToHome = function (fname) {
layoutService.addToLayoutByFname(fname).success(function() {
layoutService.getUncachedLayout().then(function(data) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 line 81 to show it is a sub-scope of line 81.


var addToLayoutByFname = function addToLayoutByFname(fname){
var tabName = SERVICE_LOC.layoutTab;
return $.ajax({
Copy link
Contributor

Choose a reason for hiding this comment

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

➡️ $http.post would be preferable over $.ajax

// $sessionStorage.layout = null;
// getLayout().then(function(result){
// $sessionStorage.layout = result;
// });
Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 removing commented out code helps to focus on the remaining active code.

// $sessionStorage.layout = result;
// });
},
error: function(request, text, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 unused params

data: null,
dataType: 'json',
async: true,
success: function(request, text) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 unused params

*/
vm.addPortlet = function addPortletFunction(fname) {
$rootScope.addToLayoutByFname(fname).success(function(){
$scope.$apply(function(request, text) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 unused params

};


$rootScope.addPortletToHome = function (fname) {
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ could this be factored into a shared service rather than being applied directly to the $rootScope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've got a through rewrite of the layout service coming down the pike. Factoring a new service just for this case would be throwaway work.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was not aware of the rewrite, thanks for the info @Doug-Reed!

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a long story, but we need this in rootScope in order to grab it from within uw-frame. If we put this in a service (like we already have in layoutService), we'd have to inject it where we need to call it in frame, which would put a hard dependency on angularjs-portal from frame. This way, we can grab it from scope if it's there, and if it's not, no harm.

@Doug-Reed Doug-Reed merged commit 454386b into master Sep 15, 2017
@Doug-Reed Doug-Reed deleted the addFromFeatureButton branch September 15, 2017 14:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants