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

[RTE] Change layout based on plain text / rich text mode #7621

Merged
merged 2 commits into from
Nov 23, 2022

Conversation

jmartinesp
Copy link
Member

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

  • Creates a ConstraintSet change when the rich text mode of the editor is enabled/disabled.
  • Disables the full screen button when either the rich text mode is disabled or it's in landscape orientation, since there is almost no vertical space available.

Motivation and context

Fixes #7620.

Screenshots / GIFs

Before After
IMAGE 2022-11-21 09:31:59 Screen

Tests

  • Enabled the Rich Text Editor in Labs.
  • Enter a room, make sure your device is in portrait orientation (vertical).
  • Enter full screen mode using the 'maximise' button.
  • Tap on the "+" button, disable formatting. Check that the full screen mode was cancelled and there is no 'maximise' button anymore.
  • Enable formatting again, the maximise button should be back.
  • Enter full screen mode again.
  • Rotate the screen to landscape. Check that the full screen mode was disabled and there is no maximise button again.

Tested devices

  • Physical
  • Emulator
  • OS version(s): 13

Checklist

@jmartinesp jmartinesp requested review from a team and bmarty and removed request for a team November 21, 2022 08:52
@@ -191,8 +197,7 @@ class RichTextComposerLayout @JvmOverloads constructor(
}

views.composerFullScreenButton.apply {
// There's no point in having full screen in landscape since there's almost no vertical space
isInvisible = resources.configuration.orientation == Configuration.ORIENTATION_LANDSCAPE
Copy link
Member

Choose a reason for hiding this comment

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

What about tablets here? Maybe add a boolean in the resource to take care of that?

@@ -254,6 +259,33 @@ class RichTextComposerLayout @JvmOverloads constructor(
views.richTextComposerEditText.isVisible = isTextFormattingEnabled
views.richTextMenu.isVisible = isTextFormattingEnabled
views.plainTextComposerEditText.isVisible = !isTextFormattingEnabled

// The layouts for formatted text mode and plain text mode are different, so we need to update the constraints
val dp = { px: Int -> (px * resources.displayMetrics.density).toInt() }
Copy link
Member

Choose a reason for hiding this comment

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

You could use DimensionConverter

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Just 2 small remarks, else LGTM.

@jmartinesp jmartinesp force-pushed the fix/modify-rte-layout-based-on-format-mode branch from 920489a to e9a99d8 Compare November 23, 2022 16:07
@jmartinesp jmartinesp enabled auto-merge (squash) November 23, 2022 16:08
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@jmartinesp jmartinesp merged commit 452cfd3 into develop Nov 23, 2022
@jmartinesp jmartinesp deleted the fix/modify-rte-layout-based-on-format-mode branch November 23, 2022 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RTE] Fix plain text mode designs
2 participants