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

[13.0][OU-MIG] website_event #2588

Merged
merged 1 commit into from
Mar 15, 2021
Merged

Conversation

MiquelRForgeFlow
Copy link
Contributor

# NOTHING TO DO

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@@ -0,0 +1,35 @@
---Models in module 'website_event'---
---Fields in module 'website_event'---
website_event / event.event / cover_properties (text) : NEW hasdefault
Copy link
Member

Choose a reason for hiding this comment

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

Are the default cover properties the equivalent visualization to what Odoo did in v12, or you should change them to mimic the previous behavior for not having visual differences?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know.

Copy link
Member

Choose a reason for hiding this comment

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

Then you should investigate it. If not, the PR is not complete.

Copy link
Contributor Author

@MiquelRForgeFlow MiquelRForgeFlow Mar 15, 2021

Choose a reason for hiding this comment

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

I investigated and I think it's fine as it is, nothing to do. That thing it's more like of a new feature, only used for background-images.

Copy link
Member

Choose a reason for hiding this comment

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

Then a blog cover in v12 is seen the same in v13, no matter the properties set in v12?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

event_cover_properties = json.loads(self.cover_properties)
# background-image might contain single quotes eg `url('/my/url')`
res['default_opengraph']['og:image'] = res['default_twitter']['twitter:image'] = event_cover_properties.get('background-image', 'none')[4:-1].strip("'")

that's the only use of event cover_properties, the twitter image, and in v12 I don't see that was filled, so it's pretty something new.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

OK, thanks for confirming

@pedrobaeza pedrobaeza merged commit 0ccd7c0 into OCA:13.0 Mar 15, 2021
@pedrobaeza pedrobaeza deleted the 13.0-mig-website_event branch March 15, 2021 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants