search

sugarlabs / sugar-web

Components for Sugar web activities
Apache License 2.0
13 stars 32 forks source link

Fix the activity popup bug in home view #66

Closed surajgillespie closed 11 years ago

surajgillespie commented 11 years ago

Since we're already setting the metadata in the activitypalette.js, this is not needed and causing a problem in the popup in the home view.

manuq commented 11 years ago

Can you tell why that code is causing the problem? This is the error that appears in shell.log when an icon is hovered in the home view:

Traceback (most recent call last):
  File "/home/manuq/prog/sugar-build/build/out/install/lib/python2.7/site-packages/sugar3/graphics/palettewindow.py", line 1298, in __enter_notify_event_cb
    self.notify_mouse_enter()
  File "/home/manuq/prog/sugar-build/build/out/install/lib/python2.7/site-packages/sugar3/graphics/palettewindow.py", line 977, in notify_mouse_enter
    self._ensure_palette_exists()
  File "/home/manuq/prog/sugar-build/build/out/install/lib/python2.7/site-packages/sugar3/graphics/palettewindow.py", line 972, in _ensure_palette_exists
    palette = self.parent.create_palette()
  File "/home/manuq/prog/sugar-build/build/out/install/lib/python2.7/site-packages/jarabe/desktop/favoritesview.py", line 461, in create_palette
    palette = FavoritePalette(self._activity_info, self._journal_entries)
  File "/home/manuq/prog/sugar-build/build/out/install/lib/python2.7/site-packages/jarabe/desktop/favoritesview.py", line 555, in __init__
    menu_item = PaletteMenuItem(text_label=entry['title'],
KeyError: 'title'
manuq commented 11 years ago

I don't know if this is related or another bug: in activitypalette.js, we have a variable "environment" that will always be undefined, inside this if statement:

https://github.com/sugarlabs/sugar-web/blob/3ddf50f9b532ffbfa4cfcfe790b4869bd95243cd/graphics/activitypalette.js#L72

manuq commented 11 years ago

Looking at datastoreObject.setMetadata and its spec, it looks possible to call it many times with different metadata.

I think ActivityPalette should only set 'title', not 'activity' or 'activity_id'. Those should be managed by activity.js .

@dnarvaez what do you think?

manuq commented 11 years ago

Hmm ActivityPalette creates another datastore.DatastoreObject(). I think this is wrong.

surajgillespie commented 11 years ago

Regarding patch, the very first time the activity is opened, metadata is set without a title. I don't know how that is causing the problem, but looking at the journal, an untitled entry is created the very first time a web activity is opened. Removing that part seems to fix the bug.

Then undefined variable "environment", i just tried to output the value in the console and it isn't undefined.

About new DS object, so if only one DS object should exist then we'll have to send the DS object from activity.js to here and work with that?

dnarvaez commented 11 years ago

I have not looked at the patch or the issue but to answer manuq question... Yes, the idea is that you only put in the object the properties you want to change. It's a bit unusual but I think requiring to set all the properties would be a pain with an async API. Need to document the datastore API...

dnarvaez commented 11 years ago

About the ds object, you can just activity.getDatastoreObject() instead of creating one.

surajgillespie commented 11 years ago

Initially I did try activity.getDatastoreObject() , but for some inexplicable reason I wasn't able to fetch the activity object using

define(["sugar-web/graphics/palette", "sugar-web/datastore", "sugar-web/env", "sugar-web/activity/shortcut", "sugar-web/activity/activity", "sugar-web/bus"], function (palette, datastore, env, shortcut, activity, bus) {

"activity" was always undefined.

dnarvaez commented 11 years ago

Perhaps a circular dependency

http://requirejs.org/docs/api.html#circular

surajgillespie commented 11 years ago

You're right. activity.js depends on activitypalette.js and vice versa.

I'll try to sort it out.

dnarvaez commented 11 years ago

Closing since it needs work.