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

InstagramPage (.s_instagram_page) #4209

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

blse-odoo
Copy link

No description provided.

@robodoo
Copy link

robodoo commented Mar 19, 2025

This PR targets the un-managed branch odoo-dev/odoo:master-mysterious-egg, it needs to be retargeted before it can be merged.

@FrancoisGe FrancoisGe force-pushed the master-mysterious-egg branch from 3ee87bf to 98b990a Compare March 19, 2025 09:22
@blse-odoo blse-odoo force-pushed the master-mysterious-egg-4-blse branch 3 times, most recently from dce7d9e to 01a2181 Compare March 24, 2025 14:15
Comment on lines 38 to 45
clean_for_save_handlers: ({ root }) => {
for (const el of root.querySelectorAll(".s_instagram_page iframe")) {
el.remove();
}
},

Choose a reason for hiding this comment

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

Why we need to remove it ? Who is adding this node

Copy link
Author

Choose a reason for hiding this comment

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

It's added by the interaction. I don't know why it is not clean up before the save. But if I don't clean it here, there are two iframes after the save

Your instagram page must be public to be integrated into an Odoo website.
</div>
<BuilderRow label.translate="Instagram Page">
<BuilderTextInput dataAttributeAction="'instagramPage'" action="'instagramPageAction'"/>

Choose a reason for hiding this comment

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

why not do every thing in instagramPageAction ? I m not sure it s a good idea to use dataAttributeAction in this case. If the apply order change it will be broken. Could you add 1 or 2 tests ?

Copy link
Author

Choose a reason for hiding this comment

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

Indeed that is a good idea to put it all in instagramPageAction, I'll do that

Copy link
Author

Choose a reason for hiding this comment

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

(so I did not add tests for this)

@blse-odoo blse-odoo force-pushed the master-mysterious-egg-4-blse branch 5 times, most recently from f2221f6 to a98f0d7 Compare March 26, 2025 12:39
@blse-odoo blse-odoo force-pushed the master-mysterious-egg-4-blse branch from a98f0d7 to 7b1d402 Compare March 26, 2025 16:26
@blse-odoo blse-odoo force-pushed the master-mysterious-egg-4-blse branch from 7b1d402 to 0005c9f Compare March 26, 2025 18:01
@FrancoisGe FrancoisGe merged commit 12e117d into master-mysterious-egg Mar 27, 2025
@FrancoisGe FrancoisGe deleted the master-mysterious-egg-4-blse branch March 27, 2025 08:13
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.

3 participants