Fix: Right panel keeps showing chat when unmaximizing widget.

[manancodes]

Checklist

• Tests written for new code (and old code if feasible)
• Linter and other CI checks pass
• Sign-off given on the changes (see [CONTRIBUTING.md] (https://github.com/matrix-org/matrix-react-sdk/blob/develop/CONTRIBUTING.md))

Fixes: element-hq/element-web#26265
Signed-off-by: Manan Sadana [email protected]

This was the behaviour before the fix:
Unmaximizing the widget does not close the Right panel, resulting in 2 chat views

And this is the behaviour after the fix:
Unmaximizing closes the Right panel.

Type: defect

---

Here's what your changelog entry will look like:

🐛 Bug Fixes

• Fix: Right panel keeps showing chat when unmaximizing widget. (#11697). Fixes Double chat (main + side) when un-maximising a widget with side chat element-hq/element-web#26265. Contributed by @manancodes.

[t3chguy]

Tests written for new code (and old code if feasible)

You checked this but actually wrote no tests..?

[manancodes]

Tests written for new code (and old code if feasible)

You checked this but actually wrote no tests..?

Sorry, corrected now.
Did not realise I had to write tests for this. I'm still getting familiar with testing, looking into this. If you could refer some resources, it would really help.

Also, there must be tests which check if the chat opens in Right Panel when widget is maximised. Where can I find that? It will help me with writing the new tests. Thanks.

[andybalaam]

Did not realise I had to write tests for this. I'm still getting familiar with testing, looking into this. If you could refer some resources, it would really help.

There's some general guidance at https://github.com/matrix-org/matrix-react-sdk/blob/develop/docs/cypress.md

Also, there must be tests which check if the chat opens in Right Panel when widget is maximised. Where can I find that? It will help me with writing the new tests. Thanks.

Take a look at cypress/e2e/right-panel/right-panel.spec.ts and the other tests in that directory. Hopefully they are similar to what you need. You might possibly need to add some new Cypress commands, similar to cy.openRoomSettings(); which is fiddly but can be done. Take a look at how that one is made.

This might be quite difficult at first, so do please ask questions in #element-dev:matrix.org . It will be easiest to help if you have some code in a branch or PR that you can point at, so your questions are specific.

Thanks!

[manancodes]

I'm still learning cypress, I have some questions:

1. Am I using the right selectors?
2. I'm sure this is not enough, how can I check if the widget is taking the whole height when maximising?
3. It checks if the right panel is opened, how can I check if the right panel has the chat in it?
4. Are the tests in the right file? If no, where should I put them?

[manancodes]

@florianduros Can you please look into this?

[florianduros]

@florianduros Can you please look into this?

I'll take a look today or tomorrow

[manancodes]

Also, when you get time, can you please answer the queries that I had?

[manancodes]

Is it ready to be merged?
I'm participating in hacktoberfest so it will help if this gets merged soon. Thanks.

[t3chguy]

@manancodes you should re-request review

[florianduros]

Also, when you get time, can you please answer the queries that I had?

• With your last changes yes!
• Let me check how this part of the code is made
• They seems to be in the right file.

[florianduros]

@manancodes

I think we can check if the chat/timeline is in the right panel with this piece of code:

this.context.rightPanelStore.currentCard.phase === RightPanelPhases.Timeline

[manancodes]

Thanks. I will add this.

[manancodes]

@florianduros

This is how it is being checked in another file

matrix-react-sdk/cypress/e2e/right-panel/right-panel.spec.ts

Lines 44 to 46 in fd08e74

	const checkRoomSummaryCard = (name: string): Chainable<JQuery<HTMLElement>> => {
	cy.get(".mx_RoomSummaryCard").should("have.length", 1);
	return cy.get(".mx_BaseCard_header").should("contain", name);

How about using something like:
cy.get(".mx_BaseCard_header").should("contain", "Chat");

I am new to cypress, implementing your suggestion will take longer because I could not find a similar implementation. I will work on that if you are not ok with my proposal.

[florianduros]

@manancodes I thought you asked how to check in the component implementation, not in the test. My fault.

cy.get(".mx_BaseCard_header").should("contain", "Chat"); is not specific to the right panel, I think it will get the main chat too.

We need first to select the right panel and query the chat in it:

cy.get(".mx_RightPanel").within(() => {
    cy.get(".mx_BaseCard_header").should("contain", "Chat"));
})

Cypress documentation within

[manancodes]

Perfect, I will add this.

[manancodes]

Done. Added and tested.

[manancodes]

@florianduros , sorry for disturbing you, but I'm participating in hacktoberfest, if this get's merged soon it will really help me with that.

[florianduros]

Thanks!