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.

Sign up for GitHub

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

Jump to bottom

[UPD] Image popup owl integration #5

Open
wants to merge 3 commits into
base: 16.0
Choose a base branch
from
Open

[UPD] Image popup owl integration #5

wants to merge 3 commits into from

Conversation

vnikolayev1
Copy link

  • moved image popup to owl

@vnikolayev1
[UPD] Image popup owl integration
9288799 
- moved image popup to owl
@katyukha
Copy link
Contributor

Hi,

Thanks for porting this module to version 16.

Currently i have following concerns about this pull request:

  • Please, create simple test module with UI, that could allow to easilly test how this module works. Or possibly easier way, to add to test_crnd_web_models module simple model and views to show how this module works.
  • ensure that this migrated version of module works fine with Odoo's image.mixin approach, when there are few copies of image with different sized.
  • may be it have sense to extend functionality of already existing Odoo image widget, that already can zoom image on hover in popover. Possibly it have sense to add popup option, that will enable image popup on click. This way it will be possible keep all standard functionality improved.

katyukha
katyukha reviewed Jul 14, 2023
View reviewed changes
crnd_web_widget_popup_image/__manifest__.py Outdated
Comment on lines 24 to 25
'crnd_web_widget_popup_image'
'/static/src/js/image_popup_owl.xml',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, move this file to static/src/xml directory.

crnd_web_widget_popup_image/static/src/js/image_popup_owl.js Outdated
import { registry } from "@web/core/registry";
import { standardFieldProps } from"@web/views/fields/standard_field_props";

export class MyNewWidget extends Component {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename widget to ImagePopup.

And, possibly, extend standard Odoo's image widget, instead of creating completely new one.

@vnikolayev1
[UPD] Image popup owl integration
a6bfea6 
- small fixes
- test module
@vnikolayev1
[UPD] Image popup owl integration
c4122fb 
- test module formview update
- lint fix
- README update
@vnikolayev1
Copy link
Author

Hey,

  • I've created test_crnd_web_widget_popup_image module for testing because test_crnd_web_models is broken
    You are able to test module from contacts.
  • I've applied image_popup for all image variations in tree view, didn't get any problems so far.
  • I did not do third part because it looked like we needed popups not for all images.

p.s. Thank you for Odoo helper. Using it since Kitworks' 2019 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@katyukha katyukha katyukha left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vnikolayev1 @katyukha