Skip to content

Mvvm RoomSummaryCard#29674

Merged
florianduros merged 14 commits into
element-hq:developfrom
tchapgouv:29633-mvvm-roomsummarycard
May 15, 2025
Merged

Mvvm RoomSummaryCard#29674
florianduros merged 14 commits into
element-hq:developfrom
tchapgouv:29633-mvvm-roomsummarycard

Conversation

@MarcWadai
Copy link
Copy Markdown
Collaborator

@MarcWadai MarcWadai commented Apr 3, 2025

Task #29633

Mvvm the roomsummarycard that has been split using two VM and moved into subfolder right_panel:

TODO

  • Create RoomSummaryCard ViewModel
  • Update RoomSummaryCard to use ViewModel
  • Unit test RoomSummaryCard and RoomSummaryCardViewModel

@MarcWadai MarcWadai requested a review from a team as a code owner April 3, 2025 16:59
@github-actions github-actions Bot added the Z-Community-PR Issue is solved by a community member's PR label Apr 3, 2025
@MarcWadai MarcWadai marked this pull request as draft April 3, 2025 17:00
Copy link
Copy Markdown
Member

@MidhunSureshR MidhunSureshR left a comment

Choose a reason for hiding this comment

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

Looks great so far 👍

Comment thread src/components/viewmodels/rooms/RoomSummaryCardViewModel.tsx Outdated
Comment thread src/components/views/right_panel/RoomSummaryCard.tsx
Copy link
Copy Markdown
Member

@florianduros florianduros left a comment

Choose a reason for hiding this comment

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

Good first draft, keep going!

I think this PR is trying to convert two components instead of one: RoomTopic & RoomSummaryCard. Can you make two differents PR for it?

Comment thread src/components/viewmodels/rooms/RoomSummaryCardViewModel.tsx
Comment thread src/components/viewmodels/rooms/RoomSummaryCardViewModel.tsx
Comment thread src/components/viewmodels/rooms/RoomSummaryCardViewModel.tsx Outdated
Comment thread src/components/viewmodels/rooms/RoomSummaryCardViewModel.tsx Outdated
Comment thread src/components/viewmodels/rooms/RoomSummaryCardViewModel.tsx Outdated
@MarcWadai
Copy link
Copy Markdown
Collaborator Author

@florianduros I moved out the changes about roomTopic into this PR

Comment thread src/components/viewmodels/rooms/RoomSummaryCardViewModel.tsx
Comment on lines +91 to +98
useEffect(() => {
for (const [, dmRoomList] of Object.entries(directRoomsList)) {
if (dmRoomList.includes(room?.roomId ?? "")) {
setDirectMessage(true);
break;
}
}
}, [room, directRoomsList]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we do setDirectMessage(false) if we can't find the room in the dmRoomList?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it is necessary since the state is reinitialized and reloaded for every new roomrummarycard displayed

@florianduros
Copy link
Copy Markdown
Member

Maybe update the PR description too :)

@florianduros florianduros added this pull request to the merge queue May 15, 2025
Merged via the queue into element-hq:develop with commit b07225e May 15, 2025
29 checks passed
snowping pushed a commit to Novaloop-AG/element-web that referenced this pull request Jun 22, 2025
* feat: create roomsummarycard viewmodel

* feat: use roomsummurycard vm in component

* test: jest unit RoomSummaryCard and RoomSummaryCardViewModel

* chore: rename to roomsummarycardview

* feat: reput room topic without vm

* test: roomSummaryCard and roomSummaryCardVM tests

* chore: add comments on roomsummarycardVM

* fix: merge conflict with roomsummarytopic, and move to vm right_panel

* fix(roomsummarycard): remove usetransition for search update

* fix: merged file that should be deleted

* fix: roomsummurycard not well merge with roomtopic

* test: update snapshots
Dileep9999 pushed a commit to hemanth-nag/element-web that referenced this pull request Oct 8, 2025
* feat: create roomsummarycard viewmodel

* feat: use roomsummurycard vm in component

* test: jest unit RoomSummaryCard and RoomSummaryCardViewModel

* chore: rename to roomsummarycardview

* feat: reput room topic without vm

* test: roomSummaryCard and roomSummaryCardVM tests

* chore: add comments on roomsummarycardVM

* fix: merge conflict with roomsummarytopic, and move to vm right_panel

* fix(roomsummarycard): remove usetransition for search update

* fix: merged file that should be deleted

* fix: roomsummurycard not well merge with roomtopic

* test: update snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Task Tasks for the team like planning Z-Community-PR Issue is solved by a community member's PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants