[10.0][MIG] web_widget_darkroom: Migrated to 10.0 #595

kellylougheed · 2017-04-01T19:24:06Z

Work in progress

lasley · 2017-08-14T21:21:14Z

@obulkin please review

lasley · 2017-08-14T21:21:44Z

web_widget_darkroom/__manifest__.py

+    'name': 'Web DarkroomJS Image Editing',
+    'summary': 'Provides web widget for image editing and adds it to standard'
+               ' image widget as modal',
+    'version': '10.0.1.0.1',


I think usually it is 10.0.1.0.0

lasley · 2017-08-14T21:22:39Z

web_widget_darkroom/static/lib/darkroomjs/core/darkroom.js

@@ -0,0 +1,356 @@
+/**
+*    Copyright 2013 Matthieu Moquet
+*    Copyright 2016-2017 LasLabs Inc.


We don't hold a copyright to this

I added the LasLabs copyrights as I wound up having to make a lot of minor changes in all the DarkroomJS files to satisfy our linter.

lasley · 2017-08-14T21:23:04Z

web_widget_darkroom/static/lib/darkroomjs/core/darkroom.js

+*    Copyright 2013 Matthieu Moquet
+*    Copyright 2016-2017 LasLabs Inc.
+*    License MIT (https://opensource.org/licenses/MIT)
+**/


Hmmm I wonder what version darkroom this is? We should update then add the version number into the header

The version is listed in the README, but I don't know if it makes sense to include the version number here as the code has been modified to satisfy our linter and wouldn't exactly match the original source for this version number.

Yeah it's still best to have it here - it's relevant in the context of the code to know what version you're dealing with.

Also hmmm I wonder if we should ignore static/lib from our linter. We don't really want to have to modify libraries like this.

@lasley FYI our linter is ignoring static/lib folder, you can see that we are not emitting message from this folder:
https://travis-ci.org/OCA/web/jobs/264532412#L638

Nice. I could swear this wasn't the case when I worked on this module in v9, but I was linting everything locally, so maybe I just missed this exception.

Thanks for the confirmation @moylop260!

…rop and pan functionality by modifying crop and zoom plugins and Darkroom widget * Add Darkroom modal to normal image widget, using darkroom.modal wizard model to provide backend support for modal view * Remove res.users view changes introduced for demo purposes (not needed due to modal functionality) * Clean up existing code, removing many unnecessary DarkroomJS files

lasley · 2017-08-16T17:58:06Z

web_widget_darkroom/views/assets.xml

+    <template id="assets_darkroom" name="web_widget_darkroom Assets" inherit_id="web.assets_backend">
+        <xpath expr="//script[last()]" position="after">
+            <link href="/web_widget_darkroom/static/src/less/darkroom.less" rel="stylesheet" type="text/less"/>
+            <script src="https://cdnjs.cloudflare.com/ajax/libs/fabric.js/1.5.0/fabric.require.min.js"/>


Bring this into static/lib/js/fabric.js and source from there instead.

obulkin

There are a couple missing migration changes. I did not review the v9 code as I was the last person to work on it. I also could not perform a functional review since the widget won't load in v10 without the Fabric.js change requested here.

obulkin · 2017-08-17T20:48:56Z

web_widget_darkroom/wizards/darkroom_modal.py

+# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html).
+
+from openerp import api, fields, models
+from openerp.exceptions import MissingError


These imports need to use odoo.

obulkin · 2017-08-17T21:30:40Z

web_widget_darkroom/static/src/js/widget_darkroom.js

+    var common = require('web.form_common');
+    var session = require('web.session');
+    var utils = require('web.utils');
+    var _ = require('_');


This line can be dropped. It doesn't work in v10, and _ should just be available as a global.

hughesbm · 2017-09-06T22:33:32Z

I have made the requested/suggested changes. I also updated some css classes that apparently changed in v10.

obulkin · 2017-09-07T03:38:14Z

My previous comments have been addressed, but when I test the widget on Runbot, I'm running into a weird cropping bug. The grey background layer created during cropping is moving together with the cropping reticle rather than staying put:

I can't seem to recreate this in v9 or in the sample on the DarkroomJs page, so I think it's an issue specific to this branch.

lasley · 2017-09-07T15:56:10Z

It seems that the desync is being caused in the click event. Clicking again synchronizes it (at least in my case)

hughesbm · 2017-09-11T21:31:13Z

The cropping bug was caused by a change to Fabric.js in version 1.7.0 that is not compatible with DarkroomJS. Downgrading to 1.6.7 appears to have restored normal functionality.

lasley · 2017-09-11T21:40:26Z

Can you add the MattKetmo/darkroomjs#112 to the ReadMe under known issues please

lasley

LGTM

obulkin

The version change does address the cropping bug in my last comment, so this should be good to go now.

rami-wafaie

Review + Functional testing: Looks fine to me
👍

rami-wafaie · 2017-10-02T15:00:05Z

@pedrobaeza can you please have a look at this??

lasley · 2017-10-02T15:40:44Z

@hughesbm - please squash for merge

* Remove fabric.js CDN link, source from static/lib instead * Update python imports (openerp -> odoo) * Remove unnecessary require('_') * Update overlay image classes to v10 * Fix typo in readme * Change module version number to standard (10.0.1.0.0) * Add DarkroomJS version number to lib files * Downgrade fabric.js (1.7.15 -> 1.6.7) * Add Darkroomjs issue to ReadMe

lasley · 2017-10-02T15:44:14Z

Oh derp you're one of mine! Squashed myself

Signed-off-by pedrobaeza

kellylougheed force-pushed the 10.0-mig-web_widget_darkroom branch from a3a2c36 to f754cdc Compare April 1, 2017 19:29

lasley added the needs review label Aug 14, 2017

lasley added this to the 10.0 milestone Aug 14, 2017

lasley suggested changes Aug 14, 2017

View reviewed changes

lasley and others added 3 commits August 14, 2017 14:52

web_widget_darkroom: Add o2m DarkroomJS widget

112eba3

[10.0][MIG] web_widget_darkroom: Migrated to 10.0

cf8f89d

lasley force-pushed the 10.0-mig-web_widget_darkroom branch from f754cdc to cf8f89d Compare August 14, 2017 21:52

lasley reviewed Aug 16, 2017

View reviewed changes

obulkin suggested changes Aug 17, 2017

View reviewed changes

lasley mentioned this pull request Aug 30, 2017

[ADD] web_widget_slick: Add module #706

Merged

hughesbm mentioned this pull request Sep 1, 2017

[ADD] web_widget_slickroom: Add module #715

Merged

2 tasks

lasley approved these changes Sep 11, 2017

View reviewed changes

obulkin approved these changes Sep 14, 2017

View reviewed changes

rami-wafaie approved these changes Oct 2, 2017

View reviewed changes

lasley force-pushed the 10.0-mig-web_widget_darkroom branch from 607a69e to bb2b144 Compare October 2, 2017 15:44

lasley merged commit 36c62c8 into OCA:10.0 Oct 2, 2017

lasley deleted the 10.0-mig-web_widget_darkroom branch October 2, 2017 15:44

cuongnmtm pushed a commit to komit-consulting/web that referenced this pull request Dec 20, 2024

Merge PR OCA#595 into 17.0

69afa1c

Signed-off-by pedrobaeza

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10.0][MIG] web_widget_darkroom: Migrated to 10.0 #595

[10.0][MIG] web_widget_darkroom: Migrated to 10.0 #595

kellylougheed commented Apr 1, 2017 •

edited

Loading

lasley commented Aug 14, 2017

lasley Aug 14, 2017

elicoidal Aug 15, 2017

lasley Aug 14, 2017

obulkin Aug 17, 2017

lasley Aug 14, 2017

obulkin Aug 17, 2017

lasley Aug 17, 2017

moylop260 Aug 18, 2017

obulkin Aug 18, 2017

lasley Aug 18, 2017

lasley Aug 16, 2017

obulkin left a comment •

edited

Loading

obulkin Aug 17, 2017

obulkin Aug 17, 2017

hughesbm commented Sep 6, 2017

obulkin commented Sep 7, 2017

lasley commented Sep 7, 2017

hughesbm commented Sep 11, 2017

lasley commented Sep 11, 2017 •

edited

Loading

lasley left a comment

obulkin left a comment

rami-wafaie left a comment

rami-wafaie commented Oct 2, 2017

lasley commented Oct 2, 2017

lasley commented Oct 2, 2017

[10.0][MIG] web_widget_darkroom: Migrated to 10.0 #595

[10.0][MIG] web_widget_darkroom: Migrated to 10.0 #595

Conversation

kellylougheed commented Apr 1, 2017 • edited Loading

lasley commented Aug 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obulkin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hughesbm commented Sep 6, 2017

obulkin commented Sep 7, 2017

lasley commented Sep 7, 2017

hughesbm commented Sep 11, 2017

lasley commented Sep 11, 2017 • edited Loading

lasley left a comment

Choose a reason for hiding this comment

obulkin left a comment

Choose a reason for hiding this comment

rami-wafaie left a comment

Choose a reason for hiding this comment

rami-wafaie commented Oct 2, 2017

lasley commented Oct 2, 2017

lasley commented Oct 2, 2017

kellylougheed commented Apr 1, 2017 •

edited

Loading

obulkin left a comment •

edited

Loading

lasley commented Sep 11, 2017 •

edited

Loading