Page MenuHomePhabricator

[web] update the keyserver selection modal based on the connection status
ClosedPublic

Authored by ginsu on Nov 28 2023, 8:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 6:46 PM
Unknown Object (File)
Fri, Nov 22, 6:33 PM
Unknown Object (File)
Fri, Nov 22, 2:22 PM
Unknown Object (File)
Fri, Nov 8, 6:07 PM
Unknown Object (File)
Fri, Nov 8, 6:03 PM
Unknown Object (File)
Fri, Nov 8, 5:45 PM
Unknown Object (File)
Fri, Nov 8, 5:29 PM
Unknown Object (File)
Fri, Nov 8, 12:39 PM
Subscribers

Details

Summary

In the desgins for the keyserver selection modal, the text of the keyserver remove info changes based on if you are currently connected to the selected keyserver or not. This diff introduces this functionality to the keyserver selection modal

For context here are the figma designs:
connected:

Screenshot 2023-11-28 at 11.18.16 PM.png (1×1 px, 129 KB)

disconnected:

Screenshot 2023-11-28 at 11.18.22 PM.png (940×1 px, 130 KB)

Linear task: https://linear.app/comm/issue/ENG-5497/introduce-remove-keyserver-ui

Test Plan

Please see the screenshots below

connected:

Screenshot 2023-11-28 at 11.18.38 PM.png (1×3 px, 767 KB)

disconnected:

Screenshot 2023-11-28 at 11.18.42 PM.png (1×3 px, 780 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.Nov 28 2023, 8:32 PM
web/modals/keyserver-selection/keyserver-selection-modal.react.js
31 ↗(On Diff #33965)

I feel like this should either be

You may delete offline keyservers from your keyserver list

or

You may delete an offline keyserver from your keyserver list

Just my opinion though (cc @ashoat)

web/modals/keyserver-selection/keyserver-selection-modal.react.js
31 ↗(On Diff #33965)

Thanks @rohan, you're right that the original sentence here is broken English. I wish @ginsu had caught it before putting up the diff. I appreciate that you brought it to my attention either way.

Let's go with @rohan's second suggestion. @ginsu, going forward please take copy a bit more seriously. I recognize that this screen is not visible outside of staff currently, but I highly doubt that anybody would have taken a deeper pass before we launch it more broadly.

It's your responsibility to check copy and I expect that you'll do better going forward, whether that means using something like Grammarly, ChatGPT, or simply reading each word out carefully.

Historically we've had a requirement that I review all copy. Given the team continues to struggle with English copywriting it might be a good idea to reinstate that requirement.

web/modals/keyserver-selection/keyserver-selection-modal.react.js
31 ↗(On Diff #33965)

Thanks for this catch @rohan. That's my bad for not catching this sooner. I had assumed that this copy had already been reviewed/was good and just copied it from directly from the Figma, but moving forward I'll always make sure to double check/be more careful when dealing with copy

I'm pretty sure I've been clear about this before – copy received from Ted (via Figma or elsewhere) is NOT approved and should never be taken verbatim without review

This revision is now accepted and ready to land.Nov 29 2023, 6:18 PM