Support for pinned messages #49
No reviewers
Labels
No labels
Blocked
Good First Issue
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
Nexus/nexus!49
Loading…
Reference in a new issue
No description provided.
Delete branch "istalri/nexus:support_pinned_messages"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The first draft to support pinned messages. Fixes #39.
TODO:
Hope you don't mind me pushing some small cleanups to your branch ^
@ -8,6 +8,7 @@ abstract class SendEventRequest with _$SendEventRequest {required String roomId,required String type,required Map<String, dynamic> content,@Default(null) String? stateKey,Not certain where you got this from... It's not valid: https://spec.mau.fi/gomuks/rpc.html#cmd-send-event
Instead,
set_statemust be used. This will need its own function inClientController.Oh then I understood something wrong. I though it was needed for this:
Matrix Spec:
Ah, well eventually it gets to Matrix, but we actually use gomuks as an SDK essentially, so we use that to e.g. send events. You can reference their docs for a schema of all commands: https://spec.mau.fi/gomuks/rpc.html
Okay, I see. I will look into it.
Edit: Just read the gomuks spec, seems the
set_stateis straight forward. At first I was not sure why there needs to be multiple endpoints for events but I then it clicked and yeah it makes sense to separate state events from other events. Although I think it's slightly misleading to havesend_eventandset_stateand notsend_state_eventor the other way around have alsoset_reactionetc but maybe that's just me.WIP: Support for pinned messages.to WIP: Support for pinned messages@ -0,0 +50,4 @@],),),AsyncData(:final value) => ListView.builder(I would say instead of this, a
SuperSliverViewshould be used, like we do in the main chat view, and instead of an extra card, just wrapEventRendererwith anInkWellor something. And then, you don't need to settextOnlyonEventRenderer.Okay, I take it back about
SuperSliverView, the performance ofListViewseems acceptable here.I've done my other suggestion with not using
textOnly.@ -511,1 +521,3 @@endDrawer: showMembersByDefault ? null : MemberList(roomId),endDrawer: showMembersByDefault && memberListOpened.value? null: Drawer(This isn't a good way to do this, the double scaffold is better. I will push a fix.
Done.
@ -0,0 +41,4 @@return .new();}Future<void> togglePin(Event event) async {I think I'd prefer a
removePinandaddPin, separate methods. And of course this needs to be called from the context menu. The context menu should have an option of "Pin" if the event is not pinned (not present in pinned events controller), and you have permission to pin, or "Unpin" if the event IS pinned, and you have permission to pin.Yeah, makes sense, seperation of concern is a useful standard. This would make it cleaner.
@Henry-Hiles wrote in #49 (comment):
I like the changes, seeing it with rested eyes, I can say I left quite the mess. I will look at it in more detail this evening/night but it looks nice on first glance. THe double Scaffold is required for having the pin drawer above the member list on pc, not sure how this will work exactly on mobile, can I open both simultaneously?
No, because both are drawers on mobile. So, one being open hides the button to open the other.
a241530f34to81bf1a8302Note: I didn't mean to push that commit "add macos folder" to this branch, I have force pushed to remove it :)
My fault!
81bf1a8302to5fef08b88eSendEventRequestuse aContent952bda6985PinnedEventsController, make 29d565fda5897995667eto19c9479539WIP: Support for pinned messagesto Support for pinned messages