Page MenuHomePhorge
Differential D125

Add confirm popup for delete event
ClosedPublic
Actions

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
View All 10 Files
Subscribers
None

Details

Reviewers
tusooa
Group Reviewers
O1: the Kazv Project
Maniphest Tasks
T83: Confirm message deletion
Commits
rK99dfb7d74e57: Add confirm popup for delete event
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 220
Build 429: GitLab CI for kazv
Build 428: arc lint + arc unit

Event Timeline

nannanko created this revision.Jul 13 2024, 9:23 AM
Owners added a reviewer: O1: the Kazv Project.Jul 13 2024, 9:23 AM
Herald added a reviewer: servant. · View Herald TranscriptJul 13 2024, 9:23 AM

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

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

nannanko added a task: T83: Confirm message deletion.Jul 13 2024, 9:23 AM
tusooa edited the summary of this revision. (Show Details)Jul 13 2024, 9:39 AM
tusooa removed a reviewer: servant.
tusooa added a comment.Jul 13 2024, 9:43 AM

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
342
src/l10n/en/100-ui.ftl
370
src/tests/quick-tests/tst_EventView.qml
504

the property to test is now opened.

Harbormaster completed remote builds in B178: Diff 298.Jul 13 2024, 10:16 AM
nannanko requested review of this revision.Jul 13 2024, 10:16 AM
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
tusooa added inline comments.Jul 17 2024, 5:09 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.

nannanko updated this revision to Diff 337.Jul 17 2024, 7:43 PM
  • Fix translation error
  • Fix test
nannanko marked 3 inline comments as done.Jul 17 2024, 7:44 PM
Harbormaster completed remote builds in B206: Diff 337.Jul 17 2024, 7:53 PM
nannanko updated this revision to Diff 348.Jul 19 2024, 1:58 AM

Rebase onto D123

Harbormaster completed remote builds in B211: Diff 348.Jul 19 2024, 2:14 AM
nannanko updated this revision to Diff 364.Jul 22 2024, 7:19 PM

Rebase onto servant

Harbormaster completed remote builds in B216: Diff 364.Jul 22 2024, 7:32 PM
nannanko updated this revision to Diff 365.Jul 23 2024, 5:30 AM
  • Use component for delete event confirm popup
  • Add event content for delete event confirm popup
  • Fix test
Harbormaster completed remote builds in B217: Diff 365.Jul 23 2024, 5:43 AM
tusooa requested changes to this revision.Jul 23 2024, 6:04 PM
tusooa added inline comments.
src/contents/ui/RoomTimelineView.qml
108–122

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
nannanko marked 2 inline comments as done.Jul 23 2024, 11:18 PM
nannanko updated this revision to Diff 376.Jul 25 2024, 8:25 AM
  • Use eventId instead of eventView for signal deleteEventRequested
Harbormaster failed remote builds in B220: Diff 376!Jul 25 2024, 8:26 AM
nannanko planned changes to this revision.Jul 26 2024, 9:21 AM
nannanko requested review of this revision.Jul 26 2024, 9:33 AM
nannanko marked an inline comment as done.
Harbormaster completed remote builds in B220: Diff 376.Jul 26 2024, 9:40 AM
tusooa added inline comments.Jul 26 2024, 12:55 PM
src/contents/ui/RoomTimelineView.qml
107–125

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
nannanko updated this revision to Diff 388.Jul 28 2024, 12:42 AM
  • Distinguish between events and local echo, add tests
  • rebase onto servant
nannanko marked an inline comment as done.Jul 28 2024, 12:55 AM
Harbormaster completed remote builds in B232: Diff 388.Jul 28 2024, 12:58 AM
nannanko planned changes to this revision.Jul 28 2024, 1:34 AM
nannanko updated this revision to Diff 389.Jul 28 2024, 1:42 AM
  • 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
110

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
tusooa added inline comments.Jul 28 2024, 4:28 PM
src/contents/ui/RoomTimelineView.qml
110

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

nannanko updated this revision to Diff 394.Aug 2 2024, 11:21 PM
  • Use eventId instead of timeline index
Harbormaster failed remote builds in B238: Diff 394!Aug 2 2024, 11:27 PM
nannanko added a parent revision: D156: Add Room::localEcho and related test.Aug 2 2024, 11:56 PM
nannanko marked 2 inline comments as done.
nannanko mentioned this in D143: Enable upload file for reply.Aug 3 2024, 7:25 PM
tusooa requested changes to this revision.Aug 7 2024, 6:56 PM
tusooa added inline comments.
src/tests/quick-tests/tst_RoomTimelineView.qml
200–202 ↗(On Diff #394)

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

This revision now requires changes to proceed.Aug 7 2024, 6:56 PM
nannanko added inline comments.Aug 9 2024, 10:00 PM
src/tests/quick-tests/tst_RoomTimelineView.qml
200–202 ↗(On Diff #394)

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

my mistake.

nannanko updated this revision to Diff 426.Aug 9 2024, 10:17 PM
  • Remove test code that delete local echo failed
nannanko marked an inline comment as done.Aug 9 2024, 10:24 PM
Harbormaster failed remote builds in B258: Diff 426!Aug 9 2024, 10:34 PM
tusooa accepted this revision.Aug 11 2024, 6:39 PM
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
Closed by commit rK99dfb7d74e57: Add confirm popup for delete event. · Explain Why
This revision was automatically updated to reflect the committed changes.
nannanko added a commit: rK99dfb7d74e57: Add confirm popup for delete event.

Revision Contents
Changeset List

Diff 376

View Options

src/contents/ui/Bubble.qml

Loading...
View Options

src/contents/ui/RoomTimelineView.qml

Loading...
View Options

src/l10n/cmn-Hans/100-ui.ftl

Loading...
View Options

src/l10n/en/100-ui.ftl

Loading...
View Options

src/tests/quick-tests/tst_EventView.qml

Loading...