Feature to add background image in SharedNotes activity (Issue #799)

[sarthak-g]

Fixed #799

demo_sharedNotes_background_addition.mp4

[llaske]

Nice work. It's a good start.
I think something goes wrong with the zoom (see below).

Plus I'm not sure why you changed the content of sugar-web directory. May be your dev branch is not up to date?

[sarthak-g]

zoom_sharedNotes_demo.mp4

I've added files in sugar-web directory for journal functionality (journalchooser.js).

[llaske]

Good job.

Two remarks:

• I'm not sure we should adapt the initial size to the screen size (cover option), I think it's better to use the original image size
• How is it possible to remove the background? May be we could add a custom sheet in Journal popup with all standard colors. So the user could choose to go back to a white background or fill it with another color. See Curriculum activity here to understand how it works.

[sarthak-g]

@llaske

• Removed cover option
• Added button to add standard colour to background and save it into the journal.

backgroundColor_SharedNotes.mp4

[llaske]

Hmmm. Removing cover doesn't seems to change anything.
When including an Abecedarium (image size 210x210) it take all the screen.
Could you try others options?

Plus it seems that the image is not drawn at the top left of the current screen.

Regarding color selection in a new sheet, good job.
Two remarks anyway:

• It will be better to localize the new sheet at the second position (just after the Journal sheet)
• Could you localize color names?

[sarthak-g]

@llaske made changes. Please review.

backgroundSharedNotes.mp4

[llaske]

Good work @sarthak-g

The image has now the right size. But it's always at top/left start position. It should be at top/left of the current position. See below.

Another issue: the share action no longer work. I guess the handling of presence should be updated to take into account the new background stuff.

[sarthak-g]

The image has now the right size. But it's always at top/left start position. It should be at top/left of the current position. See below.

I've kept that to keep the background in synchronization with the initial pan shift but I've updated in latest commit.

Another issue: the share action no longer work. I guess the handling of presence should be updated to take into account the new background stuff.

Latest commit resolved it.
If background image is added in shared mode then should it be visible on all screens?

[llaske]

I've kept that to keep the background in synchronization with the initial pan shift but I've updated in latest commit.

Nice. It works now.

Latest commit resolved it.

Hmmm. Don't works on my side. See below.

If background image is added in shared mode then should it be visible on all screens?

Yes background should be in sync on all screens.

[sarthak-g]

@llaske
Latest commit resolves

• error in sharing
• synchronization of background

See below:

background_sharedMode_sharedNotes.mp4

[sarthak-g]

Right now in shared mode, if one note is moved then it is reflected on connected users but movement of screen(i.e pan event) does not reflect on another screen. See below:

pan_background_sharedMode_sharedNotes.mp4

therefore I've not enabled movement of image on another screen. Do I have to do that?

[llaske]

Each user should be able to move its view but the background should be at the same place on every view. So I guess that you should pass the x-y shift from top/left (zoom?) to be sure to rightly set the background for each user.

Plus, you've got updated the journalchooser.js file. You must not, it should be the same for all activities.

[sarthak-g]

@llaske
In latest commit:

• background image is at same point on all shared screens and when load existing instance
• Shift background color code from journalchooser.js to new file

[llaske]

Nice work.

One more issue: Journal entries created before your PR can't be open now.

[sarthak-g]

One more issue: Journal entries created before your PR can't be open now.

@llaske , it was because of change in datastore object. I've updated it in latest commit.

[llaske]

Good.

One more thing is missing: the background is not there when the content is exported to the Journal as a PNG.

[sarthak-g]

demo_png.mp4

@llaske , it is properly working now.
It will also work for older browser versions despite of use of promise in html2canvas because of loading of es6-promise polyfill before html2canvas library.

[llaske]

Cool.
BTW you don't need to use es6-promise because ES6 syntax is now supported on all platforms. See #826

[sarthak-g]

Great, removed file in latest commit.

[llaske]

Great job. Thanks a lot for your work and your perseverance.