[17.0][MIG] web_chatter_position: Migration to 17.0 #2893

trisdoan · 2024-07-25T02:18:08Z

Note

Supersedes [17.0][MIG] web_chatter_position: Migration to 17.0 #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

With attachment viewer

Without attachment viewer

Option Side

Without attachment viewer

Rad0van · 2024-07-25T10:02:48Z

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 · 2024-07-26T15:34:31Z

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 · 2024-07-29T02:20:22Z

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 · 2024-07-29T06:52:14Z

@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 · 2024-07-30T08:32:57Z

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 · 2024-07-30T08:38:07Z

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 · 2024-07-30T08:42:55Z

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.

web_chatter.mp4

tva-subteno-it · 2024-07-31T08:15:59Z

@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 · 2024-07-31T10:09:11Z

web_chatter_position/static/src/js/web_chatter_position.esm.js

+                isInFormSheetBg: "false",
+                isChatterAside: "true",


This change set conditionally the values in order to display correctly the chatter on the bottom when the screen is too narrow to display it on the side.

Suggested change

isInFormSheetBg: "false",

isChatterAside: "true",

isInFormSheetBg: `__comp__.uiService.size < ${SIZES.XXL}`,

isChatterAside: `__comp__.uiService.size >= ${SIZES.XXL}`,

tva-subteno-it · 2024-07-31T10:09:46Z

web_chatter_position/static/src/js/web_chatter_position.esm.js

+                isChatterAside: "true",
+            });
+            setAttributes(chatterContainerHookXml, {
+                "t-attf-class": "o-aside",


This change set conditionally the values in order to display correctly the chatter on the bottom when the screen is too narrow to display it on the side.

Suggested change

"t-attf-class": "o-aside",

"t-attf-class": `{{ __comp__.uiService.size >= ${SIZES.XXL} ? "o-aside" : "" }}`,

tva-subteno-it · 2024-08-02T08:36:51Z

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

trisdoan · 2024-08-05T01:47:58Z

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

Hello, it looks good to me, thanks

Rad0van · 2024-08-05T08:02:51Z

Still the same result for bottom:

For forms that are tall this gets even worse:

tva-subteno-it · 2024-08-07T12:19:56Z

@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 · 2024-08-15T11:33:45Z

@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 · 2024-08-16T08:23:12Z

@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 · 2024-08-16T08:29:05Z

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

Rad0van · 2024-08-16T08:35:24Z

@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 · 2024-08-21T12:14:51Z

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 · 2024-08-28T12:43:22Z

@trisdoan Can you please add it?

trisdoan · 2024-08-29T02:32:53Z

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

tva-subteno-it · 2024-08-29T07:39:08Z

Perfect, I think now this PR is ready for review

DorianMAG

Tested in runboat,
functional test ok.
LGTM
Thx for this work

mailing module - Because the chatter in mass mailing form view lie under a notebook tab, if user choose sided chatter, it will disappear

CasVissers-360ERP

Functional review, thnx!

trisdoan mentioned this pull request Jul 25, 2024

[17.0][MIG] web_chatter_position: Migration to 17.0 #2833

Open

tva-subteno-it mentioned this pull request Jul 26, 2024

[17.0] mig web chatter position #2895

Open

tva-subteno-it suggested changes Jul 31, 2024

View reviewed changes

trisdoan force-pushed the 17.0-imp-web_chatter_position branch from 3e7e6f8 to 4cc57c7 Compare August 5, 2024 01:47

trisdoan force-pushed the 17.0-imp-web_chatter_position branch from b2bb6c8 to c630e41 Compare August 29, 2024 02:32

DorianMAG approved these changes Oct 2, 2024

View reviewed changes

JuMiSanAr and others added 4 commits October 14, 2024 09:41

web_chatter_position: create module

4ddadf2

[MIG] web_chatter_position: Migration to 16.0

6ab51ec

Added translation using Weblate (Turkish)

8fb701a

Added translation using Weblate (French)

aa91af1

badbole and others added 13 commits October 14, 2024 09:41

Added translation using Weblate (Croatian)

b2cefb9

Added translation using Weblate (Spanish)

eb66cb3

Fixed JS assuming "o_form_sheet_bg" is always found

b7f527c

Added translation using Weblate (Dutch)

a098ccc

Added translation using Weblate (Portuguese (Brazil))

64d7a60

[FIX] web_chatter_position: special case of sided chatter pos in mass

eca39dd

mailing module - Because the chatter in mass mailing form view lie under a notebook tab, if user choose sided chatter, it will disappear

[BOT] post-merge updates

277d7cc

Added translation using Weblate (Catalan)

03327fd

Added translation using Weblate (Italian)

07b5330

Added translation using Weblate (German)

844b637

[IMP] web_chatter_position: pre-commit auto fixes

012b99f

[MIG] web_chatter_position: Migration to 17.0

ba21263

[IMP] web_chatter_position: improve function compile

afe9506

trisdoan force-pushed the 17.0-imp-web_chatter_position branch from c630e41 to afe9506 Compare October 14, 2024 02:42

CasVissers-360ERP approved these changes Oct 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[17.0][MIG] web_chatter_position: Migration to 17.0 #2893

[17.0][MIG] web_chatter_position: Migration to 17.0 #2893

trisdoan commented Jul 25, 2024 •

edited

Loading

Rad0van commented Jul 25, 2024

tva-subteno-it commented Jul 26, 2024

trisdoan commented Jul 29, 2024

Rad0van commented Jul 29, 2024

trisdoan commented Jul 30, 2024

Rad0van commented Jul 30, 2024

trisdoan commented Jul 30, 2024

tva-subteno-it commented Jul 31, 2024

tva-subteno-it Jul 31, 2024

tva-subteno-it Jul 31, 2024

tva-subteno-it commented Aug 2, 2024

trisdoan commented Aug 5, 2024

Rad0van commented Aug 5, 2024

tva-subteno-it commented Aug 7, 2024

Rad0van commented Aug 15, 2024 •

edited

Loading

tva-subteno-it commented Aug 16, 2024

trisdoan commented Aug 16, 2024

Rad0van commented Aug 16, 2024

tva-subteno-it commented Aug 21, 2024

tva-subteno-it commented Aug 28, 2024

trisdoan commented Aug 29, 2024

tva-subteno-it commented Aug 29, 2024

DorianMAG left a comment

CasVissers-360ERP left a comment

	"t-attf-class": "o-aside",
	"t-attf-class": `{{ __comp__.uiService.size >= ${SIZES.XXL} ? "o-aside" : "" }}`,

[17.0][MIG] web_chatter_position: Migration to 17.0 #2893

Are you sure you want to change the base?

[17.0][MIG] web_chatter_position: Migration to 17.0 #2893

Conversation

trisdoan commented Jul 25, 2024 • edited Loading

Note

Context

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

Option Side

Rad0van commented Jul 25, 2024

tva-subteno-it commented Jul 26, 2024

trisdoan commented Jul 29, 2024

Rad0van commented Jul 29, 2024

trisdoan commented Jul 30, 2024

Rad0van commented Jul 30, 2024

trisdoan commented Jul 30, 2024

tva-subteno-it commented Jul 31, 2024

tva-subteno-it Jul 31, 2024

Choose a reason for hiding this comment

tva-subteno-it Jul 31, 2024

Choose a reason for hiding this comment

tva-subteno-it commented Aug 2, 2024

trisdoan commented Aug 5, 2024

Rad0van commented Aug 5, 2024

tva-subteno-it commented Aug 7, 2024

Rad0van commented Aug 15, 2024 • edited Loading

tva-subteno-it commented Aug 16, 2024

trisdoan commented Aug 16, 2024

Rad0van commented Aug 16, 2024

tva-subteno-it commented Aug 21, 2024

tva-subteno-it commented Aug 28, 2024

trisdoan commented Aug 29, 2024

tva-subteno-it commented Aug 29, 2024

DorianMAG left a comment

Choose a reason for hiding this comment

CasVissers-360ERP left a comment

Choose a reason for hiding this comment

trisdoan commented Jul 25, 2024 •

edited

Loading

Rad0van commented Aug 15, 2024 •

edited

Loading