-
Notifications
You must be signed in to change notification settings - Fork 986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(mailserver): Show current mailserver in legacy settings #21901
fix(mailserver): Show current mailserver in legacy settings #21901
Conversation
Jenkins BuildsClick to see older builds (3)
|
88% of end-end tests have passed
Failed tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (7)Click to expandClass TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletMultipleDevice:
|
@ilmotta thanks for this PR. Please take a look at the issue below. ISSUE 1 Disabling/Enabling Status nodes switcher results in failing to send messages/accepting contact requests until reloginSteps:
Actual result:
Status-debug-logs - 2025-01-09T155901.417.zip telegram-cloud-document-2-5188662600598053459.mp4Expected result: @ilmotta I would suggest to disable switcher at all. It is currently disabled in develop. Also, I see you have removed options of adding custom node and disabling Automatic selection of store node as those options are not valid anymore and were disabled in develop (see screenshot below). Therefore it makes sense to disable |
Thank you for such detailed comments @pavloburykh
That makes sense, let's keep things at a minimum. In the end, if Wdyt? |
Thank you @ilmotta. I agree, It will be the best solution. |
12de85d
to
67329ed
Compare
@pavloburykh PR is ready to be re-tested. Now the user is only able to see the mailserver text and I removed the entire screen that used to exist when pressing on the |
@ilmotta thanks a lot for this fix, no issues from my side.
I have noticed that we do not show any toast notification confirming that the text was successfully copied but I think we can ignore it as this feature will most likely be used by QAs/devs. PR is ready for merge. telegram-cloud-document-2-5211174663090101528.mp4 |
82% of end-end tests have passed
Failed tests (9)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (46)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestFallbackMultipleDevice:
Class TestActivityMultipleDevicePR:
Class TestWalletOneDevice:
Class TestWalletMultipleDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
|
100% of end-end tests have passed
Passed tests (9)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
@pavloburykh Unfortunately the code in these screens is too far from our standards, too legacy, so I preferred to do the minimum. This other issue #21896 would be the one to rewrite the advanced settings and remove the legacy settings section, but I don't see that issue being resolved for 2.33. Thanks for helping test this PR |
Fixes #21375
Summary
A regression removed more code than we wanted in #20730 and the result was broken functionality in the Legacy settings.
This PR fixes that and we now show the current mailserver. The data is kept in sync via signals, just as the original code.
pr.webm
Review notes
I didn't care about keeping the code up to our guidelines because we expect to improve the code and the UI when resolving this other issue: #21896. I copy and pasted and removed what's unnecessary.
Areas that may be impacted
Legacy settings > Sync settings > Status nodes
Steps to test
Legacy settings > Sync settings
Status nodes
Status nodes
status: ready