Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add memberlist right-panel to Hydrogen #395

Merged
merged 106 commits into from Jul 16, 2021
Merged

Add memberlist right-panel to Hydrogen #395

merged 106 commits into from Jul 16, 2021

Conversation

MidhunSureshR
Copy link
Contributor

No description provided.

@MidhunSureshR MidhunSureshR force-pushed the memberlist branch 2 times, most recently from a823d68 to a90defb Compare June 25, 2021 16:22
@bwindels
Copy link
Contributor

For #403

bwindels
bwindels reviewed Jul 1, 2021
View reviewed changes
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

This is looking great! I tried it out and the sorting and disambiguation seemed to work quite well.
I came across a few things in this first review, see the comments.

src/domain/session/leftpanel/LeftPanelViewModel.js Outdated Show resolved Hide resolved
src/domain/session/rightpanel/comparator.js Outdated Show resolved Hide resolved
src/domain/session/rightpanel/disambiguator.js Outdated Show resolved Hide resolved
src/domain/session/rightpanel/disambiguator.js Outdated Show resolved Hide resolved
src/domain/session/rightpanel/disambiguator.js Outdated Show resolved Hide resolved
src/domain/session/rightpanel/disambiguator.js Outdated Show resolved Hide resolved
src/domain/session/rightpanel/disambiguator.js Outdated Show resolved Hide resolved
src/platform/web/ui/session/rightpanel/LoadingView.js Outdated Show resolved Hide resolved
src/platform/web/ui/session/rightpanel/MemberListView.js Outdated Show resolved Hide resolved
src/matrix/room/BaseRoom.js Outdated Show resolved Hide resolved
@bwindels bwindels marked this pull request as ready for review July 1, 2021 09:46
MidhunSureshR
MidhunSureshR commented Jul 3, 2021
View reviewed changes
src/domain/session/rightpanel/MemberListViewModel.js
}
const updater = (vm, params, newMember) => {
vm.updateFrom(newMember);
this.nameDisambiguator.disambiguate(vm);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noticed something here. MappedMap won't run the mapper in onRemove. This will cause any user who leaves and rejoins a room to appear disambiguated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, can you check for this in the mapper when it is run in onAdd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed for now but there are related issues. For instance if user-A and user-B are disambiguated and user-B leaves the room, we'd ideally want user-A to be un-disambiguated (which we can't really do atm because the disambiguator does not get informed when user-B leaves).

Copy link
Contributor

Choose a reason for hiding this comment

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

good point, I think this is fine though for now.

bwindels
bwindels reviewed Jul 5, 2021
View reviewed changes
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

Nice work so far, left some comments on the lazy list view.

src/platform/web/ui/general/LazyListView.js Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Outdated Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Outdated Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Outdated Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Outdated Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Outdated Show resolved Hide resolved
bwindels
bwindels reviewed Jul 14, 2021
View reviewed changes
src/matrix/room/BaseRoom.js Outdated Show resolved Hide resolved
src/matrix/room/BaseRoom.js Outdated Show resolved Hide resolved
src/matrix/room/BaseRoom.js Show resolved Hide resolved
src/matrix/room/BaseRoom.js Outdated Show resolved Hide resolved
bwindels
bwindels reviewed Jul 15, 2021
View reviewed changes
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

looking good, we're almost there!

src/domain/session/leftpanel/LeftPanelViewModel.js Outdated Show resolved Hide resolved
src/platform/web/ui/session/rightpanel/RightPanelView.js Outdated Show resolved Hide resolved
src/platform/web/ui/css/layout.css Outdated Show resolved Hide resolved
src/platform/web/ui/css/right-panel.css Outdated Show resolved Hide resolved
src/domain/session/rightpanel/MemberListViewModel.js Outdated Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Outdated Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Outdated Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Show resolved Hide resolved
src/platform/web/ui/general/LazyListView.js Outdated Show resolved Hide resolved
src/platform/web/ui/session/rightpanel/LoadingView.js Outdated Show resolved Hide resolved
@MidhunSureshR
Copy link
Contributor Author

MidhunSureshR commented Jul 15, 2021 •
edited

TODO: Rebase against master to incorporate avatar fix and anything else.

@MidhunSureshR
Basic barebones of memberlist view
69237fc 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Duplicate navigation from room details
7e6e4ec 
- TODO: Remove duplication

Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Move rightpanel to separate view and vm
b74e445 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Modify navigation to work with rightpanel segment
7500bbe 
- Ensure that rightpanel does not show in the URL.
- Create an action of details to insert the rightpanel segment.
- Make sure rightpanel can be a child of room.

Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Create RightPanel from SessionViewModel
8b01ca5 
- Also remove old methods used in RoomDetails

Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Accommodate "rightpanel" navigation segment
dfe7385 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Modify CSS to reflect changes
1418645 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add memberlist to navigation
46a6cf6 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Remove code from session view/vm
f3c7ab3 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Create memberlist view from rightpanel
a765d7f 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Remove unused param
abd2c19 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Make list scrollable
7e72d57 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Make member private
ddb7a16 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Make getUserLevel() public
f7a6fbd 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Getter for powerlevel from room
cb5e598 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add jsdoc for powerlevels
e406aa6 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Remove comment
960f2c2 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Subscribe to powerLevels
6079379 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Move spinner before the text
5c0c590 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Css fixes
fe18b61 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Clear margin on ul
c4c0e02 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
More css fixes
4bac98d 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Remove top padding of room details view
9a3d7e4 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Remove TemplateView
0ac3d37 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Move files to members directory
da733f9 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Move LoadingView.js
1f67aa3 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Inline method
694b627 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Use ifView instead of mapView
4946683 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Use flex instead of setting height
c410aed 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Inline method
ec8b6f9 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Log instead of throwing error
611524c 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Inline method
0e0976c 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Fix lazylist
829830c 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Improve code
0bd1d2b 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Fix move bug
f366479 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Replace get with iterator and remove lambda
9fdfebf 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Make code clearer
66d5f4d 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Fix disambiguator
ec4a783 
- Do not disambiguate name on room rejoin

Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Initialize prop in constructor
436e875 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Fix layout of details panel in mobile
86bb56a 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Fix width for smaller screens
7c9755d 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Import from AvatarView.js
1a721fe 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@bwindels
Copy link
Contributor

Fantastic work, this is good to go!

@bwindels bwindels merged commit da7dff1 into element-hq:master Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwindels bwindels bwindels left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MidhunSureshR @bwindels