Page MenuHomePhorge

Add confirm popup for delete event
ClosedPublic

Authored by nannanko on Jul 13 2024, 9:23 AM.
Tags
None
Referenced Files
F104522: D125.1730676408.diff
Sat, Nov 2, 4:26 PM
F104521: D125.1730676404.diff
Sat, Nov 2, 4:26 PM
F104520: D125.1730676400.diff
Sat, Nov 2, 4:26 PM
F104519: D125.1730676396.diff
Sat, Nov 2, 4:26 PM
F104420: D125.1730666687.diff
Sat, Nov 2, 1:44 PM
F104235: D125.1730662107.diff
Sat, Nov 2, 12:28 PM
F104231: D125.1730661506.diff
Sat, Nov 2, 12:18 PM
F104217: D125.1730659127.diff
Sat, Nov 2, 11:38 AM
Subscribers
None

Details

Summary

This add a confirm overlay for event deletion.

Type: add

Test Plan

Delete an event, verify that the confirm overlay appears.

Diff Detail

Repository
rK kazv
Branch
nannanko/stacked
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 232
Build 452: GitLab CI for kazv
Build 451: arc lint + arc unit

Event Timeline

Please add a changelog type to the summary, using the following format:

Type: (add|remove|skip|security|fix)

tusooa removed a reviewer: servant.

We should also add a prompt for the user to optionally enter a reason for the deletion.

src/contents/ui/Bubble.qml
57โ€“67

use a Component for it. Instantiating OverlaySheets are expensive, so you should only instantiate it on demand. See the other popup for an example.

src/l10n/cmn-Hans/100-ui.ftl
343
src/l10n/en/100-ui.ftl
371
src/tests/quick-tests/tst_EventView.qml
504

the property to test is now opened.

tusooa requested changes to this revision.Jul 17 2024, 4:14 PM
This revision now requires changes to proceed.Jul 17 2024, 4:14 PM
src/contents/ui/Bubble.qml
57โ€“67

Well, I think we'd better display the content of the event in the popup...

See https://iron.lily-is.land/D131 for an example.

  • Fix translation error
  • Fix test
  • Use component for delete event confirm popup
  • Add event content for delete event confirm popup
  • Fix test
tusooa requested changes to this revision.Jul 23 2024, 6:04 PM
tusooa added inline comments.
src/contents/ui/RoomTimelineView.qml
129โ€“143

Don't accept an eventView as the property because it can be destroyed at any time, or to refer to another event. Pass the event id directly, and use the EventView inside the overlay to redact it.

This revision now requires changes to proceed.Jul 23 2024, 6:04 PM
  • Use eventId instead of eventView for signal deleteEventRequested
nannanko marked an inline comment as done.
src/contents/ui/RoomTimelineView.qml
128โ€“146

test this logic

tusooa requested changes to this revision.Jul 26 2024, 5:50 PM
This revision now requires changes to proceed.Jul 26 2024, 5:50 PM
  • Distinguish between events and local echo, add tests
  • rebase onto servant
  • Use index instead of eventId, fix cant display local echo in confirm popup
tusooa requested changes to this revision.Jul 28 2024, 3:21 PM
tusooa added inline comments.
src/contents/ui/RoomTimelineView.qml
131

Index is NOT reliable. It can change when new messages come in. Only use event id and txn id, which are invariant.

This revision now requires changes to proceed.Jul 28 2024, 3:21 PM
src/contents/ui/RoomTimelineView.qml
131

This is exactly the same reason why I told you not to pass an EventView: because the EventView depends on the index.

  • Use eventId instead of timeline index
tusooa requested changes to this revision.Aug 7 2024, 6:56 PM
tusooa added inline comments.
src/tests/quick-tests/tst_RoomTimelineView.qml
197โ€“199

but deleting local echo can't fail....

This revision now requires changes to proceed.Aug 7 2024, 6:56 PM
src/tests/quick-tests/tst_RoomTimelineView.qml
197โ€“199

but deleting local echo can't fail....

my mistake.

  • Remove test code that delete local echo failed
This revision is now accepted and ready to land.Aug 11 2024, 6:39 PM
This revision was landed with ongoing or failed builds.Aug 13 2024, 4:07 PM
This revision was automatically updated to reflect the committed changes.