Rich Text Composer: On iPhone in Landscape mode the fullscreen toggle should be hidden #7097
Conversation
…he positioning of the strikethrough and underline, and the maxCompressed size in landscape mode is always adapted to be visible.
…ted a constraint issue when restored during the animation for dismissal
Codecov ReportBase: 11.74% // Head: 11.73% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #7097 +/- ##
===========================================
- Coverage 11.74% 11.73% -0.01%
===========================================
Files 1620 1620
Lines 159179 159199 +20
Branches 64743 64763 +20
===========================================
+ Hits 18688 18689 +1
- Misses 139855 139874 +19
Partials 636 636
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
LGTM
But I think we can improve this further, if you look at Composer.swift line 125. We are already hiding this button depending on plainTextMode (which is actually in sync with the Element-iOS viewModel's textFormattingEnabled) and we are also forcing the minimise when toggling in WysiwygInputToolbarView.swift line 346
What I think we can do is handling both the textFormattingEnabled and the isLandscapeIphoneon the ComposerViewModel and expose a property that computes both into something like forcedMinimise (with a better name maybe) that means the button is hidden and we should minimise if it was previously maximised.
That way the Composer view is "dumber" and just responds to a single property telling it what it should do with this button 🙂
| @@ -33,20 +31,25 @@ struct Composer: View { | |||
| @Environment(\.theme) private var theme: ThemeSwiftUI | |||
|
|
|||
| @State private var isActionButtonShowing = false | |||
| @State private var isToggleButtonHidden = false | |||
|
|
|||
| private var isLandscapeIphone: Bool { | |||
There was a problem hiding this comment.
I fear Apple is gonna reject our application because of this capital 'I' 
|
@aringenbach Actually your suggestions made a lot of sense, I think I have improved the code quite a lot in this way, may you give another quick re-review of the changes? |
|
Kudos, SonarCloud Quality Gate passed!
|
Velin92
deleted the
mauroromito/fullscreen_mode_hidden_on_landscape_iphone
branch
Also moved the position of underline and strikethrough to match the design and improved the landscape sizing, when the view is compressed.
Also I removed a constraint reference from the XIB, that was probably causing a freezing when minimising in some cases.