Skip to content
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

Fix the activity popup bug in home view #66

Closed
wants to merge 1 commit into from
Closed

Fix the activity popup bug in home view #66

wants to merge 1 commit into from

Conversation

surajgillespie
Copy link
Contributor

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
Copy link
Contributor

manuq commented Jul 17, 2013

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
Copy link
Contributor

manuq commented Jul 17, 2013

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:

"activity": environment.bundleId,

@manuq
Copy link
Contributor

manuq commented Jul 17, 2013

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
Copy link
Contributor

manuq commented Jul 17, 2013

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

@surajgillespie
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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

@surajgillespie
Copy link
Contributor Author

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
Copy link
Contributor

Perhaps a circular dependency

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

@surajgillespie
Copy link
Contributor Author

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

I'll try to sort it out.

@dnarvaez
Copy link
Contributor

Closing since it needs work.

@dnarvaez dnarvaez closed this Jul 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants