Refactor various popups #16430
Refactor various popups #16430
Conversation
iurimatias
requested review from
glitchminer,
a team,
jrainville and
igor-sirotin
and removed request for
a team
Jenkins BuildsClick to see older builds (6)
|
ui/app/mainui/Popups.qml
Outdated
| isContact: details.isContact, | ||
| trustStatus: details.trustStatus, | ||
| isBlocked: details.isBlocked | ||
| } |
There was a problem hiding this comment.
I think you can just do
details.publicKey = publicKeyInstead of redefining the object.
ui/app/mainui/Popups.qml
Outdated
| @@ -240,7 +269,16 @@ QtObject { | |||
| let details = contactDetails ?? Utils.getContactDetailsAsJson(publicKey, false) | |||
| const popupProperties = { | |||
| publicKey: publicKey, | |||
| contactDetails: details | |||
| // contactDetails: details | |||
|
Another point; we used to have all these popups integrated into this SB page:
However, after the recent refactorings, this ProfileDialogView page is no longer working at all (including the popups touched here). Perhaps it would be a better idea to fix this page instead as part of the refactor work |
|
Test plan to start pending code review: https://ethstatus.testrail.net/index.php?/plans/view/18128 |
It's a small PR actually, it looks big because it adds a lot of storybook pages, and the components are all related due to a common component and dividing this into separate PRs would severely complicate things |
|
to elaborate, CommonContactDialog uses contactDetails, and that affects all components |
There was a problem hiding this comment.
I tried this PR and sending a contact request seems to be broken:
DBG 2024-10-07 18:06:40.660Z NewBE_callPrivateRPC topics="rpc" tid=93126 file=core.nim:27 rpc_method=wakuext_sendContactRequest
ERR 2024-10-07 18:06:40.660Z rpc response error topics="rpc" tid=93126 file=core.nim:36 err="\nstatus-go error [methodName:wakuext_sendContactRequest, code:-32000, message:send-contact-request: invalid message ]\n"
ERR 2024-10-07 18:06:40.660Z error doing rpc request topics="rpc" tid=93126 file=core.nim:39 methodName=wakuext_sendContactRequest exception="\nstatus-go error [methodName:wakuext_sendContactRequest, code:-32000, message:send-contact-request: invalid message ]\n"
ERR 2024-10-07 18:06:40.660Z an error occurred while sending the contact request topics="contacts-service" tid=93126 file=service.nim:501 msg="\nstatus-go error [methodName:wakuext_sendContactRequest, code:-32000, message:send-contact-request: invalid message ]\n"
There was a problem hiding this comment.
Issue 1
The popup to review the contact request of someone is empty:
The only warning in the logs that is a little suspicious is
WRN 2024-10-10 17:17:34.967Z qt warning topics="qt" tid=763306 text="TypeError: Cannot read property 'chatDetails' of undefined" file=qrc:/app/AppLayouts/Chat/stores/RootStore.qml:26 category=default
but it's about a chat, so it's probably unrelated.
Issue 2
I marked a user as untrustworthy, but after a second, the icon disappeared. I think this is a known issue though : #16392
Issue 3
Probably unrelated to this, but your own context menu doesn't look very good with a big space and a useless separator:
Issue 4
Also probably not related, but when someone is blocked, the context menu has a useless separator:
|
@iurimatias testing done. The first issue is the only one that is necessary to fix before merging this IMO. The other 3 are either already opened as issues or can be opened separetely. |
What does the PR do
Refactors the various popups to remove logic, make them functional and simplifies the code. Also added storybook pages for each of these components.
Affected areas
Popups affected:
Also affected but not testable right now because this feature is disabled (and likely will be removed):
Impact on end user
The behaviour of these popups should remain the same
How to test
Generally, right clicking on the user profile and select the menu item, then test if the functionality still works.
example:
expectation: the popup should contain the correct data about the user
expectation: contact should be removed
ID verification is less of a concern as this feature is disabled, no testing of this is expected.
Risk
Main risk is one of the popups listed in "Popups affected" no longer working.