-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[15.0][IMP] web_notify: action button name and close #2789
[15.0][IMP] web_notify: action button name and close #2789
Conversation
c0b534c
to
3af38cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!!
3af38cd
to
4ab9817
Compare
could you review @carlosdauden @CarlosRoca13 ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/ocabot merge minor |
This PR looks fantastic, let's merge it! |
@CarlosRoca13 The merge process could not be finalized, because command
|
4ab9817
to
c8020e8
Compare
/ocabot merge minor |
This PR looks fantastic, let's merge it! |
@CarlosRoca13 The merge process could not be finalized, because command
|
c8020e8
to
feb0dbe
Compare
/ocabot merge minor |
Hey, thanks for contributing! Proceeding to merge this for you. |
@CarlosRoca13 The merge process could not be finalized, because command
|
This PR has the |
web_notify/README.rst
Outdated
'res_id': self.id, | ||
'views': [(False, 'form')], | ||
}) | ||
action = self.env["ir.actions.act_window"]._for_xml_id('sale.action_orders') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have reduced here one space of indentation, which is not really correct and increases the diff.
web_notify/readme/USAGE.rst
Outdated
'res_id': self.id, | ||
'views': [(False, 'form')], | ||
}) | ||
action = self.env["ir.actions.act_window"]._for_xml_id('sale.action_orders') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here.
web_notify/readme/USAGE.rst
Outdated
:scale: 80 % | ||
:alt: Sample notifications | ||
|
||
You can test the behaviour of the notifications by installing this module in a demo database. | ||
Access the users form through Settings -> Users & Companies. You'll see a tab called "Test web notify", here you'll find two buttons that'll allow you test the module. | ||
|
||
.. figure:: static/description/test_notifications_demo.png | ||
.. figure:: ../static/description/test_notifications_demo.png |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to put the image on the img
folder:
.. figure:: ../static/description/test_notifications_demo.png | |
.. figure:: ../static/img/test_notifications_demo.png |
and rebuild the README (forcing it).
feb0dbe
to
a91a2e1
Compare
Changes done, let's see if merge bot is happy now (not very confident thou) |
/ocabot merge patch |
On my way to merge this fine PR! |
@pedrobaeza The merge process could not be finalized, because command
|
it failed as well 😖 |
I think the GIF is in a format that PIL doesn't load it. Can you reprocess it? |
a91a2e1
to
73299ec
Compare
Changed to png. Let's try again |
But we are losing the animation then... I was just talking about opening the GIF and saving again with a software that allows it to reprocess the GIF to remove any offending bit. |
Not sure, but wanted to give a hint, this looks like pypa/readme_renderer#304 from the readme-renderer here: https://github.com/OCA/oca-github-bot/blob/master/requirements.txt#L48C8-L48C16 |
73299ec
to
cb0b2de
Compare
- We can now set a button name to the notification action. - We can set an icon button as well. - After the button is clicked, the notification gets closed.
cb0b2de
to
a215ade
Compare
Thanks @ap-wtioit ! I just pushed changes that avoid scaling those screenshots. Hopefully it will be ok now |
/ocabot merge minor |
On my way to merge this fine PR! |
Congratulations, your PR was merged at b7c5ae2. Thanks a lot for contributing to OCA. ❤️ |
cc @Tecnativa TT47134
take a look @IT-Ideas @rousseldenis
ping @sergio-teruel