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

[IMP] web_editor: Image shapes for themes & website configurator #73938

Closed

Conversation

xO-Tx
Copy link
Contributor

@xO-Tx xO-Tx commented Jul 19, 2021

The goal of this PR is to add a "Python version" of the
shape on image feature using a controller.

Since the whole logic to apply shapes on image is JS based, we need
to use this URL (just like background shapes) when we want to add
a shape by default on images in themes.

When the configurator replaces a snippet image (with default shape),
the shape option should still be applied on the new one.

task-2593454

@robodoo
Copy link
Contributor

robodoo commented Jul 19, 2021

@C3POdoo C3POdoo added the RD research & development, internal work label Jul 19, 2021
@xO-Tx xO-Tx force-pushed the master-image-shapes-on-themes-mou branch 4 times, most recently from 3751c18 to f289c9f Compare July 26, 2021 15:06
@xO-Tx xO-Tx force-pushed the master-image-shapes-on-themes-mou branch from f289c9f to bc470af Compare July 26, 2021 15:30
Copy link
Contributor

@qsm-odoo qsm-odoo left a comment

Choose a reason for hiding this comment

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

Seems to be a good work, try to finish it even faster, I'd like to merge this today 👍 You'll have to adapt some stuff according to my only comment. Also in the second commit you say:

  • Update 'data-mimetype' on image to SVG since loadImageInfo()
    will set the original attachment mimetype on it.

-> seems useless to mention in the commit message

  • Prevent saving image as base64 when it's saved without
    any changes being made.

I don't see what part of the code is doing that? And should not it be in a separate fix commit then?

Squash both commits, they are very related IMO.

addons/web_editor/controllers/main.py Outdated Show resolved Hide resolved
@xO-Tx xO-Tx force-pushed the master-image-shapes-on-themes-mou branch from bc470af to 58fb9e0 Compare July 28, 2021 12:34
@xO-Tx
Copy link
Contributor Author

xO-Tx commented Jul 28, 2021

  • Prevent saving image as base64 when it's saved without
    any changes being made.

I don't see what part of the code is doing that? And should not it be in a separate fix commit then?

This happens when image has a shape option + image src is not a data URL (e.g. image with a saved shape option / image with shape added by theme using the Python URL).

I fixed that by adding a condition to prevent _applyOptions() from reapplying shapes (which converts image to base64) when image URL returns image + a shape applied on it.

Moved now to a separate commit since it's a general issue.

@xO-Tx xO-Tx force-pushed the master-image-shapes-on-themes-mou branch from 988ad8a to 29f3e5d Compare July 29, 2021 07:08
addons/web_editor/controllers/main.py Outdated Show resolved Hide resolved
addons/web_editor/controllers/main.py Outdated Show resolved Hide resolved
addons/web_editor/static/src/js/editor/snippets.options.js Outdated Show resolved Hide resolved
@xO-Tx xO-Tx force-pushed the master-image-shapes-on-themes-mou branch from 29f3e5d to 46eef9c Compare July 29, 2021 15:47
@qsm-odoo qsm-odoo marked this pull request as ready for review July 30, 2021 15:43
@qsm-odoo qsm-odoo force-pushed the master-image-shapes-on-themes-mou branch from 46eef9c to d78e92c Compare July 30, 2021 17:01
@qsm-odoo qsm-odoo force-pushed the master-image-shapes-on-themes-mou branch 2 times, most recently from 9034f95 to 17f7e8a Compare July 30, 2021 17:09
Copy link
Contributor

@qsm-odoo qsm-odoo left a comment

Choose a reason for hiding this comment

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

Thanks for the work, in the end I squashed the 3 commits and added the final xpath example in the commit message (the 3 commits made the review easier but they were really too related to be merged individually).

@robodoo r+

@robodoo
Copy link
Contributor

robodoo commented Jul 30, 2021

@qsm-odoo, you may want to rebuild or fix this PR as it has failed CI.

@qsm-odoo qsm-odoo force-pushed the master-image-shapes-on-themes-mou branch from 17f7e8a to 933b77a Compare July 30, 2021 17:32
The goal of this commit is to add a "Python version" of the
shape-on-image feature using a controller.

Since the whole logic to apply shapes on image is JS-based, we need
to use this URL (just like for background shapes) when we want to add
a shape by default on images in themes. When the configurator replaces a
snippet image (which the theme defines to have a shape), the shape
option should still be applied on the new image.

On the JS side, loadImageInfo() is overridden in order to mark
images (with theme default shapes) with corresponding attachment
data (original-id, original-src, mimetype).

Here is an example of the minimum xpath required to add a shape on a
snippet image in a theme:

```
<template id="s_image_text" inherit_id="website.s_image_text">
    <xpath expr="//img" position="attributes">
        <attribute name="src">/web_editor/image_shape/website.s_image_text_default_image/web_editor/solid/blob_1_solid_rd.svg?c2=o-color-1</attribute>
        <attribute name="data-shape">web_editor/solid/blob_1_solid_rd</attribute>
        <attribute name="data-original-mimetype">image/jpeg</attribute>
        <attribute name="data-file-name">s_image_text.svg</attribute>
        <attribute name="data-shape-colors">;o-color-1;;;</attribute>
    </xpath>
</template>
```

task-2593454
@qsm-odoo qsm-odoo force-pushed the master-image-shapes-on-themes-mou branch from 933b77a to 4cad1ef Compare July 30, 2021 17:36
@qsm-odoo
Copy link
Contributor

@JKE-be
Copy link
Contributor

JKE-be commented Jul 30, 2021

@robodoo override=ci/security

Code moved
Already quickly checked by odo #56479
Always limited to static files, so should be not risky
And now limited to svg

@qsm-odoo
Copy link
Contributor

@robodoo r+

robodoo pushed a commit that referenced this pull request Jul 30, 2021
The goal of this commit is to add a "Python version" of the
shape-on-image feature using a controller.

Since the whole logic to apply shapes on image is JS-based, we need
to use this URL (just like for background shapes) when we want to add
a shape by default on images in themes. When the configurator replaces a
snippet image (which the theme defines to have a shape), the shape
option should still be applied on the new image.

On the JS side, loadImageInfo() is overridden in order to mark
images (with theme default shapes) with corresponding attachment
data (original-id, original-src, mimetype).

Here is an example of the minimum xpath required to add a shape on a
snippet image in a theme:

```
<template id="s_image_text" inherit_id="website.s_image_text">
    <xpath expr="//img" position="attributes">
        <attribute name="src">/web_editor/image_shape/website.s_image_text_default_image/web_editor/solid/blob_1_solid_rd.svg?c2=o-color-1</attribute>
        <attribute name="data-shape">web_editor/solid/blob_1_solid_rd</attribute>
        <attribute name="data-original-mimetype">image/jpeg</attribute>
        <attribute name="data-file-name">s_image_text.svg</attribute>
        <attribute name="data-shape-colors">;o-color-1;;;</attribute>
    </xpath>
</template>
```

task-2593454

closes #73938

Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
@robodoo robodoo added the 14.5 label Jul 30, 2021
@robodoo robodoo closed this Jul 30, 2021
@robodoo robodoo temporarily deployed to merge July 30, 2021 21:33 Inactive
@qsm-odoo qsm-odoo deleted the master-image-shapes-on-themes-mou branch July 30, 2021 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
14.5 RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants