improved layout of verification modal#950
improved layout of verification modal#950jenniferhmartinezmejia-netizen wants to merge 1 commit into
Conversation
kevinaboos
left a comment
kevinaboos
left a comment
There was a problem hiding this comment.
Hi @jenniferhmartinezmejia-netizen, thanks for your contribution!
I left some comments below. While things generally look fine, I get the sense that you basically just copy-pasted Element's design decisions into Robrix. Please don't just directly copy Element; we're not trying to make an Element clone here, and I don't want to get accused of IP theft.
The only thing we need to actually change here in Robrix is the emoji layout. Nothing else -- the button order, messages shown in the modal body, text phrasing, etc doesn't need to change.
Please disclose what AI Agents or LLMs you used to generate this PR, and whether you manually reviewed each line. If not, please do so as part of your next commit.
| // SAS V1 always yields 7 emojis. | ||
| for index in 0 .. 7 { |
There was a problem hiding this comment.
it's both fragile and unnecessary to use C-style iteration with indexing, just use the proper Rust iter() like we did before.
| cancel_button := RobrixNegativeIconButton { | ||
| accept_button := RobrixPositiveIconButton { | ||
| align: Align{x: 0.5, y: 0.5} | ||
| padding: 15, | ||
| draw_icon.svg: (ICON_FORBIDDEN) | ||
| draw_icon.svg: (ICON_CHECKMARK) | ||
| icon_walk: Walk{width: 16, height: 16, margin: Inset{left: -2, right: -1} } | ||
| text: "Cancel" | ||
| text: "Yes" | ||
| } | ||
|
|
||
| accept_button := RobrixPositiveIconButton { | ||
| cancel_button := RobrixNegativeIconButton { | ||
| align: Align{x: 0.5, y: 0.5} | ||
| padding: 15, | ||
| draw_icon.svg: (ICON_CHECKMARK) | ||
| draw_icon.svg: (ICON_FORBIDDEN) | ||
| icon_walk: Walk{width: 16, height: 16, margin: Inset{left: -2, right: -1} } | ||
| text: "Yes" | ||
| text: "Cancel" | ||
| } |
There was a problem hiding this comment.
robrix always shows the confirm action button on the right side, and the cancel button on the left side. Kindly don't change that order.
| question := ModalBody { | ||
| margin: Inset{top: 10} | ||
| visible: false | ||
| } |
There was a problem hiding this comment.
why do we need a separate body here for the "question"? can't we just re-use the existing modal body?
| cx, | ||
| "Verification completed successfully! \ | ||
| Now you can read or send messages securely, and anyone you chat \ | ||
| with can also trust this device.", |
There was a problem hiding this comment.
did this come from Element? it looks suspiciously familiar, and we can't just lift copyrighted string from another project.
I'd say something like this:
| with can also trust this device.", | |
| "Verification completed successfully!\n\ | |
| Now you can send and receive encrypted messages, \ | |
| and everyone you chat with can trust this device is yours. |
2 participants
What does this PR do?
Replaces the plain text list of SAS verification emojis in the verification modal with a right-wrapping grid of cells, each displaying a large emoji glyph above its description. Also updates the confirmation button labels to "They match" / "They don't match" and adds a note to the success message about secure messaging.
Why was this PR needed?
Users reported the emojis were too small to compare against another device. The root cause was that all seven emojis were concatenated into a single ModalBody label at body-text size (11.5pt), with no per-emoji layout. Now that Makepad supports well-aligned right-wrap flow layouts, each emoji can be rendered as an independent cell at a larger size. See #846.
What are the relevant issue numbers?
Closes #846
Screenshots