Display verification status as a badge atop the user profile icon

[aaravlu]

issue241

[kevinaboos]

Nice job so far! Visually, the effect looks great, and that's almost exactly what I was hoping for.

Responding to your questions:

What should be transparent is not transparent, instead, what should not be transparent is transparent. What should I do to correct it?

Hmm, not exactly sure what you mean by this, because everything looks correct to me?

How can I centre the svg? I have debuged this for a long time but no idea.

The verification badge icon looks to be properly positioned, as it should ideally be on the upper right corner of the user profile image. So it looks like you're doing things correctly already, not sure why you would want to center it.

---

Now that I see the visual effect, I think we should probably switch to a different design for the icon. Instead of an outline of a shield that is transparent in the middle, we should switch to a shield icon that has a solid color fill (green/red/gray), with a solid-white inner checkmark / exclamation point / question mark, as that will be is easier for the user to read.

---

While the visual design is good, the code itself needs some improvement. I have left inline comments about that.
The main problem is that we should be using Makepad's Actions feature to inform the SDock widget about changes in Verification state. Actions are perfect for any scenario where you need to deliver a "message" to another widget. You definitely should not be using a shared RwLock for this purpose, as we do not want the main UI thread to ever have to acquire any locks.

[aaravlu]

Now that I see the visual effect, I think we should probably switch to a different design for the icon. Instead of an outline of a shield that is transparent in the middle, we should switch to a shield icon that has a solid color fill (green/red/gray), with a solid-white inner checkmark / exclamation point / question mark, as that will be is easier for the user to read.

This is equal to What should be transparent is not transparent, instead, what should not be transparent is transparent., my english is not good.

[aaravlu]

It seems that there is a bug:

robrix/src/home/spaces_dock.rs

Lines 192 to 193 in f4ffba5

	#[rust(VerificationState::Unverified)]
	verification_state: VerificationState,

The verification_state will be changed to its default value VerificationState::Unverified once app window was resized to mobile layout.
I also tried to set #[rust(VerificationState::Unverified)] to #[rust(VerificationState::Unknown)] , also in the expectation, the icon become the ? with gray.

Here is the screenshot:
Screencast from 2024-11-12 19-09-02.webm

I will send the log video to wechat cahnnel because it is a little large.

[kevinaboos]

Now that I see the visual effect, I think we should probably switch to a different design for the icon. Instead of an outline of a shield that is transparent in the middle, we should switch to a shield icon that has a solid color fill (green/red/gray), with a solid-white inner checkmark / exclamation point / question mark, as that will be is easier for the user to read.

This is equal to What should be transparent is not transparent, instead, what should not be transparent is transparent., my english is not good.

oh ok, great, then we agree! Sorry for the misunderstanding.

[kevinaboos]

It seems that there is a bug:

robrix/src/home/spaces_dock.rs

Lines 192 to 193 in f4ffba5

	#[rust(VerificationState::Unverified)]
	verification_state: VerificationState,

The verification_state will be changed to its default value VerificationState::Unverified once app window was resized to mobile layout.
I also tried to set #[rust(VerificationState::Unverified)] to #[rust(VerificationState::Unknown)] , also in the expectation, the icon become the ? with gray.
Here is the screenshot: Screencast from 2024-11-12 19-09-02.webm

I will send the log video to wechat cahnnel because it is a little large.

Well yes, this isn't a bug, it's just a missing part of your current implementation in this PR so far. This behavior is expected, since you haven't dealt with that case yet.

To fix this, you need to make sure the state of the verification badge is preserved when transitioning between the Desktop and Mobile views. If you're unsure how this works, I'd recommend reaching out to Julian, who wrote that code initially (or perhaps another intern can help you out).

[alanpoon]

Looks ok now. You can just do some code cleanup. Revert back all those non-related changed.

[ZhangHanDong]

From the code, it seems that the feature of displaying corresponding text on mouse hover has not been implemented yet ?

Then, when the user hovers over the badge/profile picture, we should display a tooltip that states the verification status, which is either "Verification unknown", "Unverified", or "Verified".

[aaravlu]

Screencast.from.2024-12-06.14-00-34.webm

[kevinaboos]

Screencast.from.2024-12-06.14-00-34.webm

Is this the latest behavior? You just posted a video with no comment, so I'm not sure what the point is.

If you're trying to illustrate that changing the adaptive view layout causes the verification state to be reset, then yes, you're correct. That just means you haven't properly handled the transfer of that widget state from the desktop UI instance of that widget to the mobile UI instance of that widget, or vice versa.

Please fix that, and then we can merge this PR in. 👍

[aaravlu]

Is this the latest behavior? You just posted a video with no comment, so I'm not sure what the point is.

If you're trying to illustrate that changing the adaptive view layout causes the verification state to be reset, then yes, you're correct. That just means you haven't properly handled the transfer of that widget state from the desktop UI instance of that widget to the mobile UI instance of that widget, or vice versa.

Thanks for reviewing.
The screenshot above wants to show the tooltip when user cursor is hovering the icon.
Verification state being resetted is a bug indeed, but the bug is not belong to this pr, it seems that some people is fixing it in makepad.
Now I have added some docs & done some little fixs.
Lmt if any style of the tooltip is not suitable.

[kevinaboos]

The screenshot above wants to show the tooltip when user cursor is hovering the icon.
Lmt if any style of the tooltip is not suitable.

Ok, if that's the point, then yes, I'd say to not use a transparent tooltip (that's very unusual) and just keep it simple and standard with a solid background. Also, the tooltip should be shown to the right of the verification badge icon, vertically aligned with the badge icon itself.

Verification state being resetted is a bug indeed, but the bug is not belong to this pr, is seems that some people is fixing it in makepad.

While it is technically possible for AdaptiveView to deal with this, I don't know if it ever will do that. Instead of relying on Makepad, you should explicitly obtain the current verification state when the widget is first loaded. You can do this by implementing the after_new_from_doc function from the LiveHook trait, which gets called by Makepad right after a Widget has been created.

You can get the verification state by doing:

    let current_verification_state = client.encryption().verification_state.get();

[aaravlu]

Screencast from 2024-12-17 13-44-40.webm
As you can see, for the mobile layout, I found the the widget Profile(U) is not a linear change at the bottom spacedock when resizing the window while keeping app in mobile layout.
I try my best to simulate a relative position to "follow the the widget Profile(U)".
Lmk if there are any other better way to make the tooltip follow widget Profile(U).

[kevinaboos]

Screencast from 2024-12-17 13-44-40.webm As you can see, for the mobile layout, I found the the widget Profile(U) is not a linear change at the bottom spacedock when resizing the window while keeping app in mobile layout. I try my best to simulate a relative position to "follow the the widget Profile(U)". Lmk if there are any other better way to make the tooltip follow widget Profile(U).

I'm not sure how to fix that, but it looks quite good for now!

[kevinaboos]

Things are looking quite good! Just left a few small comments.

[kevinaboos]

We shouldn't need two separate views, VerificationNoticeDesktop and VerificationNoticeMobile. It's inefficient and less maintainable to do it that way.

Instead, combine them into one view. Then, set the relevant states in the Profile's handle_event() functions like you already are, e.g., using the show_with_options(). There's no reason to have multiple duplicate views.

[aaravlu]

Yeah!
As you can see, I also changed the internal attributes like padding & align in <VerificationNoticeMobile>. Simply call show_with_options() without changing attributes to set absolutely position would not work correctly for mobile layout.

Thus, I am thinking how about using an <AdaptiveView> , then putting <VerificationNoticeDesktop> & <VerificationNoticeMobile> in it?

[kevinaboos]

Nah, no need for that here. An AdaptiveView isn't what we want here, since we don't want the content to differ based on view size. The need to do that right now is just based on the shortcomings of the current makepad tooltip support; in the future, the tooltip widget should handle all of that automatically.

[kevinaboos]

To clarify, I was suggesting that you just create a single VerificationNotice view, and then set the properties of it (padding, align, etc) accordingly in the Profile::handle_event() function. That's faster and easier to understand than creating two separate views and hiding/showing one of them.

[aaravlu]

NP!