Skip to content

[18.0][MIG] web_responsive: Migration to 18.0#3037

Merged
OCA-git-bot merged 219 commits intoOCA:18.0from
kobros-tech:18.0-mig-web_responsive
Apr 23, 2025
Merged

[18.0][MIG] web_responsive: Migration to 18.0#3037
OCA-git-bot merged 219 commits intoOCA:18.0from
kobros-tech:18.0-mig-web_responsive

Conversation

@kobros-tech
Copy link
Copy Markdown
Contributor

No description provided.

@chaule97
Copy link
Copy Markdown

Hi, I opened another PR #3057, welcome you review it

@kobros-tech kobros-tech force-pushed the 18.0-mig-web_responsive branch 3 times, most recently from ac1f548 to daaafd6 Compare January 24, 2025 08:55
@CarlosRoca13
Copy link
Copy Markdown
Contributor

Please, note this pr #3069

@kobros-tech kobros-tech force-pushed the 18.0-mig-web_responsive branch 3 times, most recently from 804cb3f to 786da5e Compare January 27, 2025 21:29
lasley and others added 18 commits January 28, 2025 01:31
* Add Tecnativa as author
* Remove untestable JS method
* Rename to web_responsive
* Bump version
* Change all openerp to odoo
* Fix qunit suite injection
* Remove jQuery require
* Change to new selectors:
  * `oe_leftbar` to `o_sub_menu`
  * `oe_logo` to `o_sub_menu_logo`
  * `oe_footer` to `o_sub_menu_footer`
  * `oe_secondary_menus_container` to `o_sub_menu_content`
* Add style to hide oe_footer instead of remove to not break `support_branding`
* Add note in ReadMe explaining override of `support_branding`
* Set top margin of app drawer title to 0 to fit v10 proportions
* Fix notification badge positioning
* Add o_web_client class to body to fix overlaps
* Scroll control panel with page
* Change navbar z-index to not be overlapped by buttons
* Raise z-index on header to raise over buttons
* Handle layered notifications via z-index
* Remove `#` from navigation links HREF to prevent history littering
* Rebalance z-indexes to fix overlay issue while still providing notification support
* Bold titles for apps in the app-drawer
* Remove z-index from Control Panel buttons, so it doesn't overlap the menu.
* Better responsive menus
  * Move systray icons/menus out of the menu and up to the top bar.
  * Increase avatar size and visibility.

* Other minor menu behavior fixes.
* Changes to fit odoo variables
* And a little margin adjust on navbar-right
* Top menu always thick
* Removed overflow:hidden!important from main.less
* Local patch: prevent body overflow auto
* Stop using `style=` on `<body>`
* Fix jquery drawer in anticipation of:
   blivesta/drawer#36
On screens with less than 768px content is limited to 2 columns.

On mobile field labels have their own line.

Fix elements width for XS screens
Make image editing controls always available, instead of depending
on resolution or hover.
Small makeup to make it look good in XS screens.
* [IMP] drawer-toggle

* [IMP] Hitbox of close drawer-toggle

* [IMP] Removed outline on menu item

* [IMP] Logo positionings

* [FIX] Try me on runbot button

* [REM] Reversed outline change
* Change accesskey for `edit` in form view back to `e` to fix OCA#587
Before this patch, there was an incompatibility between this addon and l10n_es_toponyms, caused by the lack of wrapping of the special address field that was being added.

Now layout works as expected in that case. In any other case, it seems sensible to wrap inputs so no more weird overflows happen and less chances of needing horizontal scrolling happen.
* FIX hasclass

* FIX views

* [FIX] web_responsive: Syntax error in xpath
@pedrobaeza
Copy link
Copy Markdown
Member

I already stated in #3057 (review) and here in #3037 (comment).

My initial comment with detailed explanation: #2405 (comment)

Extracted from there:

  1. Document viewer

    Odoo includes natively the possibility of viewing in an embedded popup PDF files, videos or images. On previous versions, as the community edition always put the chatter on the bottom, these documents were previewed full size, covering the contents and not letting you to work side by side.

    Selección_016

    web_responsive then included code for working on a more similar way than enterprise: putting the document viewer on the side, for easing the input for example on vendor bills, copying things side by side:

    Selección_017

    But it was also decided to do it in a more convenient way:

    • Don't opening by default the so considered main attachment by Odoo. This is not always what you want to do, and it covers the conversation (or move it to the bottom, being not consistent where to find it).
    • Allow to display this way any of the attachments, not only what they consider the main one.

    Odoo has now included these enterprises features in the community edition, but they use a different "document viewer" for the side view, and it still has the first mentioned flaw. So I think we need to tackle this in the OCA module. Possible options:

    1. Disable the "side viewer", and use the main document viewer in the same way as in previous versions.
    2. Don't move the chatter to bottom, don't open the side viewer in any case, opening it when clicking the attachments, and put a close X icon on the side viewer to "return" to the chatter.

See also #2745, and how it was added in 17 in #2795

@pedrobaeza
Copy link
Copy Markdown
Member

@kobros-tech I insist: the problem is the automatic display of that preview in the invoices. It should be disabled by default. We want the preview, but not automatic. Do you understand me?

@pedrobaeza
Copy link
Copy Markdown
Member

And that automatic preview is not using the same stuff:

imagen

@pedrobaeza
Copy link
Copy Markdown
Member

I am asked to disable the default one but keeping the function if I click on attachmen?

Yes. This is what we have as part of web_responsive module since a lot of versions ago, and what you have to keep migrating the module. It's not something new that is asked.

becaue of the pros of the one we see when cliking on attachment over the other auto one?

Yes, already explained in quotes, but summarizing:

  • Chatter visible without scrolling.
  • Maximize possibility.
  • Be able to see this way more than one attachment, not the main one.

@kobros-tech kobros-tech force-pushed the 18.0-mig-web_responsive branch 3 times, most recently from aefa365 to 3a8c615 Compare April 20, 2025 14:04
@pedrobaeza
Copy link
Copy Markdown
Member

/ocabot migration web_responsive

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Apr 21, 2025
Copy link
Copy Markdown
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.

Functionally tested on runboat and now the part about not auto-previewing the attachment is working correctly.

Three comments inline for being clarified before proceeding with the merge, waiting also for some confirmation of the rest of the contributors/reviewers. cc @carlos-lopez-tecnativa @chaule97

@kobros-tech kobros-tech force-pushed the 18.0-mig-web_responsive branch from 3a8c615 to 8b648c0 Compare April 21, 2025 12:38
Copy link
Copy Markdown
Contributor

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

Tested and working. Just a few comments related to the features that work fine on desktop, and I think they can be improved to work well on mobile. Please let me know if they can be improved or if they are out of the scope of this module.

Comment on lines +20 to +22
- Sticky header & footer in list view

![image](../static/img/listview.gif)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry for the delay. I uploaded a video to explain my comment. Please let me know if this case can be handled by this module or not.

sticky_header_not_working_on_mobile.mp4

t-att-data-menu-xmlid="props.app.xmlid"
t-att-href="props.href"
t-on-click="onClick"
draggable="false"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In Odoo enterprise, now it is possible to change the order of the menu items. I'm not sure if support for this can be added here, or if this attribute prevents its use.

menu.reordering.mp4

Copy link
Copy Markdown
Contributor

@carlos-lopez-tecnativa carlos-lopez-tecnativa Apr 23, 2025

Choose a reason for hiding this comment

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

Ok for me, this comment can be considered a new feature.

@kobros-tech kobros-tech force-pushed the 18.0-mig-web_responsive branch from 8b648c0 to 245f2af Compare April 21, 2025 17:34
@kobros-tech
Copy link
Copy Markdown
Contributor Author

Are we ready for migration or is there any remaining comment?

@pedrobaeza
Copy link
Copy Markdown
Member

It seems there are 2 pending comments: #3037 (comment) and #3037 (comment)

@kobros-tech
Copy link
Copy Markdown
Contributor Author

kobros-tech commented Apr 23, 2025

the two comments are not applied in version 17.0

@kobros-tech kobros-tech force-pushed the 18.0-mig-web_responsive branch from 245f2af to 1c5824d Compare April 23, 2025 13:29
@kobros-tech
Copy link
Copy Markdown
Contributor Author

Any specific migration comments?

I welcome migration related comments, but any new feature I can not add here now.

Copy link
Copy Markdown
Contributor

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

I wish the issue mentioned in this comment #3037 (comment) could be handled by this module, because it works fine on desktop, but in mobile view the header is not sticky.
But anyway, let's go!

@kobros-tech
Copy link
Copy Markdown
Contributor Author

it should be in a separate PR as the issue you mentioned is coming from old version, I see it as a feature in fact.

Copy link
Copy Markdown
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, let's go with this. Thanks for all the work.

/ocabot merge nobump

@OCA-git-bot
Copy link
Copy Markdown
Contributor

On my way to merge this fine PR!
Prepared branch 18.0-ocabot-merge-pr-3037-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 631855c into OCA:18.0 Apr 23, 2025
6 of 7 checks passed
@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 6a9a4ad. Thanks a lot for contributing to OCA. ❤️

@houssine78
Copy link
Copy Markdown

thank you @pedrobaeza for having push the review so far !

thank you @kobros-tech for this most wanted PR

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.