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.

Sign up for GitHub

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

Rich Text Composer - Maximise/Minimise toggle #6971

Merged
merged 11 commits into from
Oct 25, 2022
Merged

Rich Text Composer - Maximise/Minimise toggle #6971

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a5b128c
reimplemented but the animation and the spacing needs some fixing
Velin92 Oct 21, 2022
866721d
fixing view not returning in place
Velin92 Oct 21, 2022
63fbbce
animation
Velin92 Oct 21, 2022
254dfb6
improving anim, however it only works with swiftui build
Velin92 Oct 24, 2022
40942b1
minor adjustments
Velin92 Oct 25, 2022
be08438
speeding the animation a bit
Velin92 Oct 25, 2022
8d89b66
tests and identifier improvements
Velin92 Oct 25, 2022
672bc39
fix
Velin92 Oct 25, 2022
9219da0
changelog
Velin92 Oct 25, 2022
2fcceaa
removed unused code
Velin92 Oct 25, 2022
eae3931
comment
Velin92 Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions Riot/Modules/Room/Views/WYSIWYGInputToolbar/WysiwygInputToolbarView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class WysiwygInputToolbarView: MXKRoomInputToolbarView, NibLoadable, HtmlRoomInp
let subView: UIView = hostingViewController.view
self.addSubview(subView)

hostingViewController.view.translatesAutoresizingMaskIntoConstraints = false
self.translatesAutoresizingMaskIntoConstraints = false
subView.translatesAutoresizingMaskIntoConstraints = false
heightConstraint = subView.heightAnchor.constraint(equalToConstant: height)
NSLayoutConstraint.activate([
Expand All @@ -103,7 +103,12 @@ class WysiwygInputToolbarView: MXKRoomInputToolbarView, NibLoadable, HtmlRoomInp
.sink(receiveValue: { [weak self] idealHeight in
guard let self = self else { return }
self.updateToolbarHeight(wysiwygHeight: idealHeight)
})
}),
wysiwygViewModel.$idealHeight
.removeDuplicates()
.sink { [weak hostingViewController] _ in
hostingViewController?.view.setNeedsLayout()
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a comment to describe why this is needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes that bug that made the view stay centered and expanded when it is maximised and then minimised.
Essentially the superview does not recognize the change back to the minimised form, and does not layout again its subviews. By listening to the ideal height changes, it forces the super view to do another layout again.

Copy link
Member Author

Choose a reason for hiding this comment

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

added

}
]

update(theme: ThemeService.shared().theme)
Expand Down
39 changes: 39 additions & 0 deletions RiotSwiftUI/Modules/Room/Composer/Test/UI/ComposerUITests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,19 @@ final class ComposerUITests: MockScreenTestCase {
wysiwygTextView.typeText("test")
XCTAssertTrue(sendButton.exists)
XCTAssertFalse(app.buttons["editButton"].exists)

let maximiseButton = app.buttons["maximiseButton"]
let minimiseButton = app.buttons["minimiseButton"]
XCTAssertFalse(minimiseButton.exists)
XCTAssertTrue(maximiseButton.exists)

maximiseButton.tap()
XCTAssertTrue(minimiseButton.exists)
XCTAssertFalse(maximiseButton.exists)

minimiseButton.tap()
XCTAssertFalse(minimiseButton.exists)
XCTAssertTrue(maximiseButton.exists)
}

func testReplyMode() throws {
Expand All @@ -56,6 +69,19 @@ final class ComposerUITests: MockScreenTestCase {
let textViewContent = wysiwygTextView.value as! String
XCTAssertFalse(textViewContent.isEmpty)
XCTAssertFalse(cancelButton.exists)

let maximiseButton = app.buttons["maximiseButton"]
let minimiseButton = app.buttons["minimiseButton"]
XCTAssertFalse(minimiseButton.exists)
XCTAssertTrue(maximiseButton.exists)

maximiseButton.tap()
XCTAssertTrue(minimiseButton.exists)
XCTAssertFalse(maximiseButton.exists)

minimiseButton.tap()
XCTAssertFalse(minimiseButton.exists)
XCTAssertTrue(maximiseButton.exists)
}

func testEditMode() throws {
Expand All @@ -82,5 +108,18 @@ final class ComposerUITests: MockScreenTestCase {
let textViewContent = wysiwygTextView.value as! String
XCTAssertTrue(textViewContent.isEmpty)
XCTAssertFalse(cancelButton.exists)

let maximiseButton = app.buttons["maximiseButton"]
let minimiseButton = app.buttons["minimiseButton"]
XCTAssertFalse(minimiseButton.exists)
XCTAssertTrue(maximiseButton.exists)

maximiseButton.tap()
XCTAssertTrue(minimiseButton.exists)
XCTAssertFalse(maximiseButton.exists)

minimiseButton.tap()
XCTAssertFalse(minimiseButton.exists)
XCTAssertTrue(maximiseButton.exists)
}
}
62 changes: 35 additions & 27 deletions RiotSwiftUI/Modules/Room/Composer/View/Composer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ struct Composer: View {
viewModel.viewState.sendMode == .edit ? "editButton" : "sendButton"
}

private var toggleButtonAcccessibilityIdentifier: String {
wysiwygViewModel.maximised ? "minimiseButton" : "maximiseButton"
}

private var toggleButtonImageName: String {
wysiwygViewModel.maximised ? Asset.Images.minimiseComposer.name : Asset.Images.maximiseComposer.name
}

private var borderColor: Color {
focused ? theme.colors.quarterlyContent : theme.colors.quinaryContent
}
Expand All @@ -76,8 +84,6 @@ struct Composer: View {
var body: some View {
VStack(spacing: 8) {
let rect = RoundedRectangle(cornerRadius: cornerRadius)
// TODO: Fix maximise animation bugs before re-enabling
// ZStack(alignment: .topTrailing) {
VStack(spacing: 12) {
if viewModel.viewState.shouldDisplayContext {
HStack {
Expand All @@ -103,36 +109,39 @@ struct Composer: View {
.padding(.top, 8)
.padding(.horizontal, horizontalPadding)
}
WysiwygComposerView(
focused: $focused,
content: wysiwygViewModel.content,
replaceText: wysiwygViewModel.replaceText,
select: wysiwygViewModel.select,
didUpdateText: wysiwygViewModel.didUpdateText
)
.tintColor(theme.colors.accent)
.placeholder(viewModel.viewState.placeholder, color: theme.colors.tertiaryContent)
.frame(height: wysiwygViewModel.idealHeight)
.padding(.horizontal, horizontalPadding)
.onAppear {
wysiwygViewModel.setup()
HStack(alignment: .top, spacing: 0) {
WysiwygComposerView(
focused: $focused,
content: wysiwygViewModel.content,
replaceText: wysiwygViewModel.replaceText,
select: wysiwygViewModel.select,
didUpdateText: wysiwygViewModel.didUpdateText
)
.tintColor(theme.colors.accent)
.placeholder(viewModel.viewState.placeholder, color: theme.colors.tertiaryContent)
.frame(height: wysiwygViewModel.idealHeight)
.onAppear {
wysiwygViewModel.setup()
}
Button {
wysiwygViewModel.maximised.toggle()
} label: {
Image(toggleButtonImageName)
.resizable()
.foregroundColor(theme.colors.tertiaryContent)
.frame(width: 16, height: 16)
}
.accessibilityIdentifier(toggleButtonAcccessibilityIdentifier)
.padding(.leading, 12)
.padding(.trailing, 4)
}
// Button {
// withAnimation(.easeInOut(duration: 0.25)) {
// viewModel.maximised.toggle()
// }
// } label: {
// Image(viewModel.maximised ? Asset.Images.minimiseComposer.name : Asset.Images.maximiseComposer.name)
// .foregroundColor(theme.colors.tertiaryContent)
// }
// .padding(.top, 4)
// .padding(.trailing, 12)
// }
.padding(.horizontal, horizontalPadding)
.padding(.top, topPadding)
.padding(.bottom, verticalPadding)
}
.clipShape(rect)
.overlay(rect.stroke(borderColor, lineWidth: 1))
.animation(.easeInOut(duration: 0.1), value: wysiwygViewModel.idealHeight)
.padding(.horizontal, horizontalPadding)
.padding(.top, 8)
.onTapGesture {
Expand All @@ -148,7 +157,6 @@ struct Composer: View {
.resizable()
.foregroundColor(theme.colors.tertiaryContent)
.frame(width: 14, height: 14)

}
.frame(width: 36, height: 36)
.background(Circle().fill(theme.colors.system))
Expand Down
1 change: 1 addition & 0 deletions changelog.d/6954.change
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added the maximise/minimise toggle button to the Rich Text Composer