[17.0][MIG] web_chatter_position: Migration to 17.0

[trisdoan]

Note

• Supersedes https://github.com/OCA/web/pull/2833

Context

• Before this change, there are 2 issues

With option side, there is duplicated chatter in Invoice module

With option bottom, when attachment viewer exists

Result in Invoicing (also applies with Sale module)

Option Bottom

1. With attachment viewer

2. Without attachment viewer

Option Side

1. Without attachment viewer

[Rad0van]

This does not feel right. When chatter is at bottom, it is always visible and takes part of the screen and leaves small portion for form itself. It should be attached at the end instead.

[tva-subteno-it]

May I ask you why you created a new PR? Was it only because the other dev was not responding anymore? Because I tried your PR, and this is what it looks like when I choose "Sided" in a sale order.

I found that the previous PR was working just fine, so I don't know what where the problems that made you change its code.

[trisdoan]

May I ask you why you created a new PR? Was it only because the other dev was not responding anymore? Because I tried your PR, and this is what it looks like when I choose "Sided" in a sale order.

I found that the previous PR was working just fine, so I don't know what where the problems that made you change its code.

Hi, I explained the reason in the description of the PR.

iirc, it works fine in Sales, let me take a look.

[Rad0van]

@trisdoan, but that is not the way we work here in OCA. You could supersede PR if it is clearly abandoned for long period of time or the author of the original PR approves it. The original author hasn't answered for for few weeks but as can be seen has done some activity in June so not the former case nor the latter.

Also the superseding PR should be at least as good functionally as the superseded one but preferably better which unfortunately is not the case with your PR.

Contributions are highly welcome and valued. In this case it would be best to help finish the existing PR.

[trisdoan]

May I ask you why you created a new PR? Was it only because the other dev was not responding anymore? Because I tried your PR, and this is what it looks like when I choose "Sided" in a sale order.

I found that the previous PR was working just fine, so I don't know what where the problems that made you change its code.

Hello @tva-subteno-it, I just checked, I works fine on my side Result:

Could you please share again (nicer if you provide video), but this time please open devtools (right click -> inspect -> console)

[Rad0van]

May I ask you why you created a new PR? Was it only because the other dev was not responding anymore? Because I tried your PR, and this is what it looks like when I choose "Sided" in a sale order. I found that the previous PR was working just fine, so I don't know what where the problems that made you change its code.

Hello @tva-subteno-it, I just checked, I works fine on my side Result:

Could you please share again (nicer if you provide video), but this time please open devtools (right click -> inspect -> console)

Have you checked all the other options? I have posted a screenshot with "Bottom" selected. It's broken.

[trisdoan]

This does not feel right. When chatter is at bottom, it is always visible and takes part of the screen and leaves small portion for form itself. It should be attached at the end instead.

Hello @Rad0van, thanks for your review.

I just checked, my improve commit made the module behave like it does in version 16.

https://github.com/user-attachments/assets/bf8af5de-faa5-44d7-9233-ff75d3888cc7

[tva-subteno-it]

@trisdoan I know why it wasn't working on my side: my screen was just a bit shorter than required to display correctly. If I move my window to a bigger screen, then the chatter appears correctly on the the right side. So maybe you could look into it, in order to display it in a nicer way when the screen is too small than juste centering it at the bottom.

[tva-subteno-it]

Hi @trisdoan, what do you think about the changes I suggested?

[trisdoan]

Hi @trisdoan, what do you think about the changes I suggested?

Hello, it looks good to me, thanks

[Rad0van]

Still the same result for bottom:

For forms that are tall this gets even worse:

[tva-subteno-it]

@Rad0van are you sure you have the latest version of this PR? On my side I have no problems and it seems that from the start of this PR you are the only one with this bug.

[Rad0van]

@Rad0van are you sure you have the latest version of this PR? On my side I have no problems and it seems that from the start of this PR you are the only one with this bug.

I may have used the other PR's contents previously but I mage sure this time I chose the right one. For me everything works except for bottom. In that case the chatter is at bottom but is fixed - takes part of the screen and the main form just scrolls behind it. In 16 it was behaving differently (I no longer can see your video to compare). Here's a screenshot:

The same thing in 16:

However when in responsive mode and the chatter gets to botton it works as expected.

[tva-subteno-it]

@Rad0van I know why it's not working on your side: it's because of the web_responsive module. Please try the web_chatter_position without any additional module. If there are some incompatibilities between modules, I think the best way to handle them is to create issues directly on the concerned module. It means that for the moment web_chatter_position works fine on its own.

[trisdoan]

Thank you so much for taking care of it @tva-subteno-it

[Rad0van]

@Rad0van I know why it's not working on your side: it's because of the web_responsive module. Please try the web_chatter_position without any additional module. If there are some incompatibilities between modules, I think the best way to handle them is to create issues directly on the concerned module. It means that for the moment web_chatter_position works fine on its own.

You're right. Without web_responsive it works. Hopefully @trisdoan can make it work together. In 16 there is no issue when both are installed so I hope it can be solved.

[tva-subteno-it]

I found where the problem was: a new CSS rule was added in the web_responsive module setting the overflow of the form sheet to auto instead of being unset. So we need to unset it manually in the web_chatter_position module. Please @trisdoan, can you try creating the file form_statusbar.scss inside the scss folder, and adding the following:

.o_xxl_form_view .o_form_sheet_bg .o_form_sheet {
    overflow: unset !important;
}

It should fix the problem.

[tva-subteno-it]

@trisdoan Can you please add it?

[trisdoan]

Hello @tva-subteno-it, it's done, nice work, thank you

[tva-subteno-it]

Perfect, I think now this PR is ready for review

[fa-odoo]

hello, is it possible to use the button as on version 16.0 instead of the configuration on user preferences?

[houssine78]

@legalsylvain Could you have a look at this PR it has more than 2 reviews but the bot didn't marked it as approved. This PR also fix an conflict with web_responsive.

If this one is ok for you. Could we merged it and then we can close the duplicate PRs https://github.com/OCA/web/pull/2833 and https://github.com/OCA/web/pull/2895

thanks !

cc: @pedrobaeza

[HaraldPanten]

@legalsylvain Could you have a look at this PR it has more than 2 reviews but the bot didn't marked it as approved. This PR also fix an conflict with web_responsive.

If this one is ok for you. Could we merged it and then we can close the duplicate PRs #2833 and #2895

thanks !

cc: @pedrobaeza

@etobella do you mind managing this migration?

1- We need to close 2 opened PRs (abandoned) 2- Could we merge this PR?

[codeagencybe]

Is there anything else that is holding back merging this PR? Also been waiting quite some time for this one. I only see tva-subteno-it as reviewer requested changes? Appreciated if this one can be resolved soon.

🙏

[fanha99]

LGTM

[HaraldPanten]

@legalsylvain Could you have a look at this PR it has more than 2 reviews but the bot didn't marked it as approved. This PR also fix an conflict with web_responsive. If this one is ok for you. Could we merged it and then we can close the duplicate PRs #2833 and #2895 thanks ! cc: @pedrobaeza

@etobella do you mind managing this migration?

1- We need to close 2 opened PRs (abandoned) 2- Could we merge this PR?

@pedrobaeza Could you have a look, please?

Manage the migration Assignation and close the other 2 PRs. Merge this PR if you consider that is correct. It has several validations.

THX!

[pedrobaeza]

Well, I let other maintainers to do it, as I have a special problem with this feature, denormalizing the expected way to work. For me, the way to go is #2976 (but I would even integrate it in web_responsive).

[HaraldPanten]

Well, I let other maintainers to do it, as I have a special problem with this feature, denormalizing the expected way to work. For me, the way to go is #2976 (but I would even integrate it in web_responsive).

I understand... OK, I'll ask to others. Thanks!

@yajo @tarteo Could you manage this migration?

• Manage the migration Assignation and close the other 2 PRs.
• Merge this PR if you consider that is correct. It has several validations.

THX!

[yajo]

Sorry, I also don't maintain this. However, if somebody volunteers as a maintainer I'll merge it.

[trisdoan]

Hello @yajo, I can reserve some time for this module when needed