Skip to content
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: Remove Reset Session for MLS conversations #3298

Merged

Conversation

borichellow
Copy link
Contributor

What's new in this PR?

Issues

When we receive decryption errors in an MLS conversation, we show a “reset session” button.
But this button does not help to reset the session and is useless.

Therefore, we should hide it and not show it to the user.

Causes (Optional)

Nobody think about differences in MLS conversations :)

Solutions

Think about it and remove "Reset Session" btn and text "Try resetting the session ..." text

  • small refactoring in Compose MessageItem to make it bit more readable

@@ -240,6 +242,9 @@ internal fun MessageDecryptionFailure(
text = stringResource(R.string.label_learn_more)
)
VerticalSpace.x4()

if (conversationProtocol !is Conversation.ProtocolInfo.Proteus) return@Column
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in fact this is the only changes in Compose, all others - just refactoring to make it more readable

Copy link
Contributor

github-actions bot commented Aug 8, 2024

Test Results

860 tests   860 ✅  12m 37s ⏱️
118 suites    0 💤
118 files      0 ❌

Results for commit 2e7d1a6.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Aug 8, 2024

APKs built during tests are available here. Scroll down to Artifacts!

Copy link
Contributor

@alexandreferris alexandreferris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch on the issue 👌🏻

Just added 2 comments for better readability on ConversationTopAppBar and if we can add one more preview with decryption error

Copy link

sonarqubecloud bot commented Aug 9, 2024

Copy link
Contributor

github-actions bot commented Aug 9, 2024

APKs built during tests are available here. Scroll down to Artifacts!

@borichellow borichellow merged commit 8311bc9 into release/cycle-4.6 Aug 9, 2024
13 of 14 checks passed
@borichellow borichellow deleted the fix/remove_reset_session_for_mls_conversations branch August 9, 2024 11:57
@echoes-hq echoes-hq bot added echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. echoes: technical-roadmap/throughput More specific category, to highlight task aiming at improving the development velocity and effici... labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. echoes: technical-roadmap/throughput More specific category, to highlight task aiming at improving the development velocity and effici... size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants