Page MenuHomePhabricator
Differential D7836

[web] Displaying modal of editing message GUI
ClosedPublic
Actions

Authored by • kuba on May 16 2023, 5:40 AM.
Tags
None
Referenced Files
F3351998: D7836.diff
Sat, Nov 23, 4:21 AM
Unknown Object (File)
Mon, Nov 4, 10:42 AM
Unknown Object (File)
Mon, Nov 4, 10:42 AM
Unknown Object (File)
Mon, Nov 4, 10:42 AM
Unknown Object (File)
Mon, Nov 4, 10:42 AM
Unknown Object (File)
Mon, Nov 4, 10:42 AM
Unknown Object (File)
Mon, Nov 4, 10:42 AM
Unknown Object (File)
Sat, Nov 2, 6:45 AM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
michal
inka
Commits
rCOMMe899ba275f83: [web] Displaying modal of editing message GUI
Summary

Added displaying editing message modal in place of the original message.

Test Plan
  • Entered edit mode.
  • Checked if the background is dimmed, and the modal displayed on top of it.
  • Resized window, checked if the position was updated.
  • Exited edit mode.
  • Checked if the modal disappeared.

Screenshot:

Screenshot 2023-05-16 at 14.42.06.png (1×1 px, 106 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• kuba created this revision.May 16 2023, 5:40 AM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMay 16 2023, 5:40 AM
• kuba added a parent revision: D7833: [web] Storing edited message component position.May 16 2023, 5:40 AM
• kuba edited the test plan for this revision. (Show Details)
• kuba added a child revision: D7837: [web] Added FocusTrap for modals.May 16 2023, 5:44 AM
Harbormaster completed remote builds in B19390: Diff 26528.May 16 2023, 5:58 AM
• kuba requested review of this revision.May 16 2023, 5:58 AM
• kuba updated this revision to Diff 26644.May 18 2023, 8:12 AM

Rebase

Harbormaster completed remote builds in B19479: Diff 26644.May 18 2023, 8:34 AM
michal added inline comments.May 19 2023, 3:10 AM
web/chat/edit-message-provider.js
74 ↗(On Diff #26644)

This is obviously needed, but I don't understand why, could you explain? Isn't the width measured from a component inside of the ComposedMessage? Why is the delivery icon included in the width?

ashoat added inline comments.May 19 2023, 6:24 AM
web/chat/edit-message-provider.js
12 ↗(On Diff #26644)

Is this something we can also use for rendering the delivery icon, so that we can guarantee it is referenced / used in both places, and future maintainers are aware? Otherwise this is very brittle and hard to maintain... it's likely something changes with the delivery icon, and nobody will know that this code needs to be updated

• kuba updated this revision to Diff 26840.May 23 2023, 1:22 AM

Removed width parameter

• kuba added a comment.May 23 2023, 1:24 AM

I removed completely the width property from the EditTextMessage component. It turned out it was completely unnecessary.

Harbormaster completed remote builds in B19632: Diff 26840.May 23 2023, 1:40 AM
michal accepted this revision.May 23 2023, 4:32 AM
This revision is now accepted and ready to land.May 23 2023, 4:32 AM
Closed by commit rCOMMe899ba275f83: [web] Displaying modal of editing message GUI (authored by kosmydel <kukubaczek@gmail.com>). · Explain WhyMay 24 2023, 3:47 AM
This revision was automatically updated to reflect the committed changes.
kosmydel <kukubaczek@gmail.com> added a commit: rCOMMe899ba275f83: [web] Displaying modal of editing message GUI.

Revision Contents
Changeset List

PathSize
web/
chat/
28 lines

Diff 26528

View Options

web/chat/edit-message-provider.js

Loading...