Skip to content

Commit

Permalink
Activity palette improvements
Browse files Browse the repository at this point in the history 
- properly set and get the metadata
- fix inheritance
  • Loading branch information
Manuel Quiñones committed Jul 30, 2013
1 parent a56c419 commit ca60d81
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 54 deletions.
13 changes: 9 additions & 4 deletions activity/activity.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ define(["webL10n",

l10n.start();

datastoreObject = new datastore.DatastoreObject();

var activityButton = document.getElementById("activity-button");

var activityPalette = new activitypalette.ActivityPalette();
var activityPalette = new activitypalette.ActivityPalette(
activityButton, datastoreObject);

// Colorize the activity icon.
activity.getXOColor(function (error, colors) {
Expand All @@ -36,14 +39,16 @@ define(["webL10n",

shortcut.add("Ctrl", "Q", this.close);

datastoreObject = new datastore.DatastoreObject();

env.getEnvironment(function (error, environment) {
datastoreObject.setMetadata({
"activity": environment.bundleId,
"activity_id": environment.activityId
});
datastoreObject.save(function () {});
datastoreObject.save(function () {
datastoreObject.getMetadata(function (error, metadata) {
activityPalette.setTitleDescription(metadata);
});
});
});
};

Expand Down
82 changes: 32 additions & 50 deletions graphics/activitypalette.js
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
define(["sugar-web/graphics/palette",
"sugar-web/datastore",
"sugar-web/env",
"sugar-web/bus"], function (palette, datastore, env, bus) {
"sugar-web/env"], function (palette, env) {

var activitypalette = {};

activitypalette.ActivityPalette = function () {
activitypalette.ActivityPalette = function (activityButton,
datastoreObject) {

palette.Palette.call(this, activityButton);

var activityTitle;
var descriptionLabel;
Expand Down Expand Up @@ -38,68 +39,49 @@ define(["sugar-web/graphics/palette",
this.setContent([titleContainer, descLabelContainer,
descriptionContainer]);

var titleElem = document.getElementById("title");
var descriptionElem = document.getElementById("description");
var datastoreObject = new datastore.DatastoreObject();
var mdata;
var title;

bus.listen();

datastoreObject.getMetadata(function (error, metadata) {
mdata = metadata;
setTitleDescription();
});

setTitleDescription();

function setTitleDescription() {
if ((mdata === undefined) || (mdata.title === undefined)) {
env.getEnvironment(function (error, environment) {
title = environment.activityName;
datastoreObject.setMetadata({
"activity": environment.bundleId,
"activity_id": environment.activityId,
"title": title
});
datastoreObject.save(function () {});
titleElem.value = title;
});
} else {
if (mdata.title !== undefined) {
titleElem.value = mdata.title;
datastoreObject.setMetadata({
"activity": environment.bundleId,
"activity_id": environment.activityId,
"title": mdata.title
});
datastoreObject.save(function () {});
}

if (mdata.description !== undefined)
descriptionElem.value = mdata.description;
}
}
this.titleElem = document.getElementById("title");
this.descriptionElem = document.getElementById("description");

titleElem.onblur = function () {
this.titleElem.onblur = function () {
datastoreObject.setMetadata({
"title": this.value
});
datastoreObject.save(function () {});
};

descriptionElem.onblur = function () {
this.descriptionElem.onblur = function () {
datastoreObject.setMetadata({
"description": this.value
});
datastoreObject.save(function () {});
};
};

var activityButton = document.getElementById("activity-button");
// Fill the text inputs with the received metadata.
var setTitleDescription = function (metadata) {
var that = this;
if (metadata.title === undefined) {
env.getEnvironment(function (error, environment) {
that.titleElem.value = environment.activityName;
});
} else {
this.titleElem.value = metadata.title;
}

if (metadata.description !== undefined) {
this.descriptionElem.value = metadata.description;
}
}

activitypalette.ActivityPalette.prototype =
new palette.Palette(activityButton);
Object.create(palette.Palette.prototype, {
setTitleDescription: {
value: setTitleDescription,
enumerable: true,
configurable: true,
writable: true
}
});

return activitypalette;
});

3 comments on commit ca60d81

@surajgillespie
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the inheritance fix Manuel.
I noticed that you removed datastoreObject.setMetadata( ) part.
Shouldn't "title" be an attribute of metadata?

Right now it only sets the title in "this.titleElem"
and when i tried to output the metadata object, i didn't find the "title" field.

Coming back to the bug(popup in home view), as i said earlier,
d0f2f08
the bug is caused because we didn't set title in this commit.

Walter bender pointed out an "untitled" entry in the journal when activity is opened for the first time.
I think it is because of the same reason.

In my pull request, i had taken out that commit because we were already setting those attributes in activitypalette.js.(name, title and activity id)
Now since you have removed them from here, I could set the title part in activity.js itself,
Shall i do that??

Btw, https://github.com/sugarlabs/sugar-web/blob/master/graphics/activitypalette.js#L63
it always results in false because there is no title entry in metadata.

@manuq
Copy link
Contributor

@manuq manuq commented on ca60d81 Jul 30, 2013

Choose a reason for hiding this comment

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

Thanks for the inheritance fix Manuel.
I noticed that you removed datastoreObject.setMetadata( ) part.

No, I just removed the calls that didn't make sense. Title is set after the user leaves the title entry, and the same happens with description:

We don't need to set title or description in other situations.

Shouldn't "title" be an attribute of metadata?

Right now it only sets the title in "this.titleElem"
and when i tried to output the metadata object, i didn't find the "title" field.

Your datastore might be corrupted because of the old bug. Try with a clean ~/.sugar . You can remove the directory and it will be recreated again.

Coming back to the bug(popup in home view), as i said earlier,
d0f2f08
the bug is caused because we didn't set title in this commit.

The bug was caused because there were two datastore objects being created.

Walter bender pointed out an "untitled" entry in the journal when activity is opened for the first time.
I think it is because of the same reason.

That's a consequence of creating two datastore objects.

In my pull request, i had taken out that commit because we were already setting those attributes in activitypalette.js.(name, title and activity id)
Now since you have removed them from here, I could set the title part in activity.js itself,
Shall i do that??

My commit fixed everything. Please give it a try with a clean datastore.

Btw, https://github.com/sugarlabs/sugar-web/blob/master/graphics/activitypalette.js#L63
it always results in false because there is no title entry in metadata.

False :) See above.

@surajgillespie
Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't think the issue is solved :(

When I tested ( i did take out .sugar), the bug remains as long as "this.titleElem.onblur" isn't called.
So only when we click in the input box, " this.titleElem.onblur " comes into play and the metadata is set, otherwise it is not.
There is no metadata field title if we don't touch the input box of the activity palette.

Please sign in to comment.