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

Master snippet options owl agau #3305

Draft
wants to merge 128 commits into
base: master-snippet-options-owl-ard
Choose a base branch
from

Conversation

agau-odoo
Copy link

@agau-odoo agau-odoo commented Jun 24, 2024

@robodoo
Copy link

robodoo commented Jun 24, 2024

This PR targets the un-managed branch odoo-dev/odoo:master-snippet-options-owl-ard, it needs to be retargeted before it can be merged.

@agau-odoo agau-odoo force-pushed the master-snippet-options-owl-agau branch from 5a4c1e1 to 56dcb87 Compare June 24, 2024 13:48
Copy link

@detrouxdev detrouxdev left a comment

Choose a reason for hiding this comment

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

Already a good start, only one thing that I'm wondering about, and comment for the cherry-picked commit.

I also cherry-picked the following commits, I will take the others once the comments have been addressed:

  • [FIX] web_editor: WeColorpicker this.close() -> this.state.close()
  • [REF] website: convert s_image options to Owl
  • [FIX] web_editor: WeColorpicker this.close() -> this.state.close()

Comment on lines 65 to 60
await this.trigger_up("widgets_start_request", {
this.website.websiteRootInstance.trigger_up("widgets_start_request", {

Choose a reason for hiding this comment

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

Why was the await removed?

Copy link
Author

Choose a reason for hiding this comment

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

Because trigger_up never returns a Promise

Choose a reason for hiding this comment

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

🤦🏻 good catch. I think we should await the widgets_start_request like we do here for example. And this should probably be fixed in stable too 👍🏻

Copy link
Author

Choose a reason for hiding this comment

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

I've pushed an update, should be good!

Choose a reason for hiding this comment

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

Awesome thanks! Did you also do the fix in stable? It's probably not that big of a deal since it doesn't cause a bug, but it's still wrong and is a quick win IMO.

Copy link
Author

Choose a reason for hiding this comment

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

Oh I can do that! Thanks for telling me :) I'll try to think about this kind of thing from now on.
odoo#171166

addons/web_editor/static/src/js/editor/snippets.options.js Outdated Show resolved Hide resolved
@detrouxdev detrouxdev force-pushed the master-snippet-options-owl-ard branch from fb98eb8 to 50c6b00 Compare June 25, 2024 14:35
@agau-odoo agau-odoo force-pushed the master-snippet-options-owl-agau branch 17 times, most recently from b5777cf to 4244b8c Compare July 2, 2024 12:12
@agau-odoo agau-odoo force-pushed the master-snippet-options-owl-agau branch from 4244b8c to 0a9e0cc Compare July 3, 2024 13:27
@detrouxdev detrouxdev force-pushed the master-snippet-options-owl-ard branch from 50c6b00 to f2f5489 Compare July 3, 2024 14:27
@agau-odoo agau-odoo force-pushed the master-snippet-options-owl-agau branch 2 times, most recently from ed0113b to 593257f Compare July 4, 2024 07:21
@detrouxdev detrouxdev force-pushed the master-snippet-options-owl-ard branch 5 times, most recently from 3a541aa to 81f72e6 Compare July 8, 2024 08:57
agau-odoo and others added 10 commits August 9, 2024 11:23
This is never attached to any snippet (no selector). Even if it was, it
is impossible to move Showcase sub-elements (that start with a title and
an icon), and it would therefor have no effect.

task-3850413
*: so_snippet_addition and so_content_addition

task-3850413
*: s_newsletter_block, s_newsletter_subscribe_popup,
s_newsletter_subscribe_form

Separate snippets into their respective folders

task-3850413
*: website_blog, website_event, website_forum, website_hr_recruitment,
website_sale, website_slides

task-3850413
so_snippet_addition and so_content_addition are fully converted and can
now be removed.

The mass_mailing legacy snippet addition selector was outdated in this
commit [1].

The mass_mailing legacy content addition selector was outdated in this
commit [2]. Further updates were pushed in these commits [3] [4].

[1]: edf81c1
[2]: ee94c0c
[3]: 1dff079
[4]: 507b80a

task-3850413
This is never attached to any snippet (no selector).

task-3850413
*: all products, checkout, product, header checkout button

task-3850413
@agau-odoo agau-odoo force-pushed the master-snippet-options-owl-agau branch from 78fc54f to f40ca0d Compare August 9, 2024 09:26
@agau-odoo agau-odoo force-pushed the master-snippet-options-owl-agau branch from f9f2853 to b74f017 Compare August 12, 2024 06:41
@agau-odoo agau-odoo force-pushed the master-snippet-options-owl-agau branch from e6e21b0 to 5747608 Compare August 12, 2024 08:51
@agau-odoo agau-odoo force-pushed the master-snippet-options-owl-agau branch from 5747608 to 08a92b4 Compare August 12, 2024 08:57
@detrouxdev detrouxdev force-pushed the master-snippet-options-owl-ard branch 9 times, most recently from 633ee8d to 1240c73 Compare August 20, 2024 08:52
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.

5 participants