Modernize the ForwardMessageModal component

[hackerbirds]

"ModalHost" is replaced with the newer and simpler "Modal". The CSS is also updated to unify the theme palette

First time contributor checklist:

• I have read the README and Contributor Guidelines
• I have signed the Contributor Licence Agreement

Contributor checklist:

• My contribution is not related to translations.
• My commits are in nice logical chunks with good commit messages
• My changes are rebased on the latest main branch
• A yarn ready run passes successfully (more about tests here)
• My changes are ready to be shipped to users

Description

The "forward message" modal component was based off ModalHost and looked old and off with the rest of the app so I decided to simply the code using the newer(?) Modal component which allowed me to simplify the code and CSS a little bit. I rely on the parent CSS components for the theme so that it gets updated and looks like the other modern components in the apps.

Visually the changes are mostly color changes, as the updated Modal looks darker, and there are updated back/close buttons that match slightly better. They now have hovering effects and look nicer overall.

To be honest I am a beginner in React components so do tell me if I implemented something the wrong way that I could try to correct.

I did a bit of manual testing, in both light theme and dark theme, with images and without, and everything looks right to me. Let me know if I missed anything, I know I do sometimes.

Screenshots

I'm sorry I had to censor a bunch because I got lazy so the changes are harder to see:

BEFORE CHANGES

AFTER CHANGES

[hackerbirds]

Oops, found an issue in my code with the change in the editing mode (the Modal becomes scrollable when it shouldn't be). I'll try to fix it and let you know when it's done.

[hackerbirds]

Hi,

the issue I had was fixed.

Also, here is the the thought process behind my code changes since there's a few changes:

• <ModalHost>/<animated.div> was changed with <Modal>
• The footer element which was inside <ModalHost> is moved outside because you have to set the footer straight in <Modal ...> with modalFooter
• I removed redundant CSS which was in ForwardMessageModal.scss which was already present in Modal.scss (the parent). This should also be useful in the future for design changes, like updating the color palette or padding etc. since the changes should automatically get shared with the forward message modal (unlike before).
• By default Modal.scss allowed scrolling in the main body, but because we already had scrolling elements in ForwardMessageModal (the contact selector and the edit input element), I override this property in ForwardMessageModal.scss.
• I had found a "bug" in Modal.scss with the back button CSS (which appears to have never really been used before so it is probably why no one caught that), which still had the "hover" gray background (that the close button doesn't have; it has a blue square around the button when you hover over it). I fixed it by simply copying the "close" button css behaviour over the back button.

As for testing I did more manual testing on all the elements I could think of and didn't find any abnormal behaviour (yet..). Large message body, small message body, images or not... Again, in both light theme and dark theme the elements render how they're supposed to.

Lastly I have a question. Right now (in my implementation) the header title ("Forward to" text) is aligned with the left when selecting contacts, but align on the center when editing a message as you can see in the screenshots. That's because the back button element appears, and the space-between flex elements realign this way. Is this something you guys are OK with or that I should fix, keeping in mind that the left alignment is how other modals without back buttons in the app look like? (The old current implementation leaves the "Forward to" constantly centered)

[hackerbirds]

I made a change in <Modal> that makes onEscape call onBackButtonClick (if it's there). I thought that this should be the intended behaviour since pressing Escape is usually assumed by the user to be "go back" (it also fixed an issue I had).

To my knowledge onBackButtonClick is only used here to show a confirmation popup if something is selected. The onEscape change I made doesn't affect it or any other Modal that I have tried.

[hackerbirds]

Rebased to latest and waiting for review, when you find the time :)

[scottnonnenberg-signal]

@hackerbirds Looks like you have some lint failures - can you get those fixed? yarn ready is what you run locally to make sure things are in good shape there.

[hackerbirds]

@hackerbirds Looks like you have some lint failures - can you get those fixed? yarn ready is what you run locally to make sure things are in good shape there.

Thanks for letting me know. yarn lint should pass now (with the exception of a WhatsNew issue that doesn't seem to be related to my code?)

[scottnonnenberg-signal]

Yep, that failure in main is unrelated - I just pushed some fixes: https://github.com/signalapp/Signal-Desktop/commits/main

[hackerbirds]

Alright, I'll update and rebase again

[scottnonnenberg-signal]

Alright, thanks for all your work on this. We've merged it internally, and it will be in 6.23.x. To be specific, we took 8caaae8, not your most recent push. You can maybe revert it if you want this to auto-close when we push to the public main branch?

[hackerbirds]

Alright, thanks for all your work on this. We've merged it internally, and it will be in 6.23.x. To be specific, we took 8caaae8, not your most recent push. You can maybe revert it if you want this to auto-close when we push to the public main branch?

The push was just to rebase to the latest changes, nothing should have changed. If it was merged internally I assume I can just close this PR manually now?

[scottnonnenberg-signal]

Sure, feel free to do that. We can work with you to make sure this shows as a purple Merged status too - it's up to you.

But we'll call you out in the release notes as well, so that may be the badge of honor you're really looking for! :0)

[hackerbirds]

Don't worry about that. Thanks for your time