-
Notifications
You must be signed in to change notification settings - Fork 325
Fix the MessageMenu displaying elements outside the window after resizing it #278
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,7 @@ struct MessageMenu<MainButton: View, ActionEnum: MessageMenuAction>: View { | |
| @Binding var isShowingMenu: Bool | ||
|
|
||
| /// Overall ChatView Frame | ||
| let chatViewFrame: CGRect = UIScreen.main.bounds | ||
| @State private var chatViewFrame: CGRect = .zero | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if it's the size of the whole chat, it should be calculated on a chat view, not inside a menu. that will allow to only do it once, and not for each menu showing an important question: why use this size for calculations at all? is there probably a way to avoid this completely? for landscape check, there is an orientation environment var I believe; for width .infinity width would achieve the same result; verticalOffset of 2 screen heights probably just means off-screen etc so could you please check if this can be avoided? |
||
|
|
||
| /// The max height for the menu | ||
| /// - Note: menus that exceed this value will be placed in a ScrollView | ||
|
|
@@ -152,62 +152,75 @@ struct MessageMenu<MainButton: View, ActionEnum: MessageMenuAction>: View { | |
| } | ||
|
|
||
| public var body: some View { | ||
| ZStack(alignment: .top) { | ||
|
|
||
| // Reaction Overview Rectangle | ||
| if reactionOverviewIsVisible, case .vStack = messageMenuStyle { | ||
| ReactionOverview(viewModel: viewModel, message: message, width: reactionOverviewWidth, backgroundColor: theme.colors.messageFriendBG, inScrollView: false) | ||
| .frame(width: reactionOverviewWidth) | ||
| .maxHeightGetter($reactionOverviewHeight) | ||
| .offset(y: UIApplication.safeArea.top) | ||
| .transition(defaultTransition) | ||
| GeometryReader { geometry in | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is a frameGetter modifier in Utils in this project, could you please use that one - the syntax is easier, and you won't have to use additional indentaion also GeometryReader is expensive, so could you please make sure it only applied for ipads, since ios doesn't need this? |
||
| ZStack(alignment: .top) { | ||
|
|
||
| // Reaction Overview Rectangle | ||
| if reactionOverviewIsVisible, case .vStack = messageMenuStyle { | ||
| ReactionOverview(viewModel: viewModel, message: message, width: reactionOverviewWidth, backgroundColor: theme.colors.messageFriendBG, inScrollView: false) | ||
| .frame(width: reactionOverviewWidth) | ||
| .maxHeightGetter($reactionOverviewHeight) | ||
| .offset(y: UIApplication.safeArea.top) | ||
| .transition(defaultTransition) | ||
| .opacity(messageMenuOpacity) | ||
| } | ||
|
|
||
| // Some views to help debug layout and animations | ||
| //debugViews() | ||
|
|
||
| // The message and menu view | ||
| messageMenuView() | ||
| .frameGetter($messageMenuFrame) | ||
| .position(x: chatViewFrame.width / 2 + horizontalOffset, y: verticalOffset) | ||
| .opacity(messageMenuOpacity) | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please remove the extra \n |
||
| } | ||
|
|
||
| // Some views to help debug layout and animations | ||
| //debugViews() | ||
|
|
||
| // The message and menu view | ||
| messageMenuView() | ||
| .frameGetter($messageMenuFrame) | ||
| .position(x: chatViewFrame.width / 2 + horizontalOffset, y: verticalOffset) | ||
| .opacity(messageMenuOpacity) | ||
|
|
||
| } | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| .edgesIgnoringSafeArea(.all) | ||
| .background( | ||
| ZStack { | ||
| Rectangle() | ||
| .foregroundStyle(.ultraThinMaterial) | ||
| Rectangle() | ||
| .fill(.primary.opacity(0.1)) | ||
| } | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| .edgesIgnoringSafeArea(.all) | ||
| .opacity(backgroundOpacity) | ||
| .onTapGesture { | ||
| if viewState == .keyboard { | ||
| keyboardState.resignFirstResponder() | ||
| .background( | ||
| ZStack { | ||
| Rectangle() | ||
| .foregroundStyle(.ultraThinMaterial) | ||
| Rectangle() | ||
| .fill(.primary.opacity(0.1)) | ||
| } | ||
| .edgesIgnoringSafeArea(.all) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wrong alignment here, please make sure the formatting is neat |
||
| .opacity(backgroundOpacity) | ||
| .onTapGesture { | ||
| if viewState == .keyboard { | ||
| keyboardState.resignFirstResponder() | ||
| transitionViewState(to: .ready) | ||
| } else { | ||
| dismissSelf() | ||
| } | ||
| } | ||
| ) | ||
| .onAppear { | ||
| chatViewFrame = geometry.frame(in: .global) | ||
|
|
||
| transitionViewState(to: .prepare) | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(Int(animationDuration * 333))) { | ||
| transitionViewState(to: .original) | ||
| } | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(Int(animationDuration * 666))) { | ||
| transitionViewState(to: .ready) | ||
| } else { | ||
| dismissSelf() | ||
| } | ||
| } | ||
| ) | ||
| .onAppear { | ||
| transitionViewState(to: .prepare) | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(Int(animationDuration * 333))) { | ||
| transitionViewState(to: .original) | ||
| } | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(Int(animationDuration * 666))) { | ||
| transitionViewState(to: .ready) | ||
| .onChange(of: keyboardState.keyboardFrame) { | ||
| if viewState == .ready, keyboardState.isShown { | ||
| transitionViewState(to: .keyboard) | ||
| } | ||
| } | ||
| } | ||
| .onChange(of: keyboardState.keyboardFrame) { | ||
| if viewState == .ready, keyboardState.isShown { | ||
| transitionViewState(to: .keyboard) | ||
| .onChange(of: geometry.size) { _ , newSize in | ||
| chatViewFrame = CGRect( | ||
| origin: chatViewFrame.origin, | ||
| size: CGSize(width: newSize.width, height: newSize.height) | ||
| ) | ||
|
|
||
| // Recalculate reaction overview width | ||
| reactionOverviewWidth = chatViewFrame.width - UIApplication.safeArea.leading - UIApplication.safeArea.trailing | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -231,7 +244,7 @@ struct MessageMenu<MainButton: View, ActionEnum: MessageMenuAction>: View { | |
| reactionOverviewIsVisible = shouldShowReactionOverviewView | ||
| reactionSelectionIsVisible = shouldShowReactionSelectionView | ||
| menuIsVisible = true | ||
| verticalOffset = UIScreen.main.bounds.height * 2 | ||
| verticalOffset = chatViewFrame.height * 2 | ||
|
|
||
| /// Kick off the background animation | ||
| withAnimation(.easeInOut(duration: animationDuration)) { | ||
|
|
@@ -250,7 +263,7 @@ struct MessageMenu<MainButton: View, ActionEnum: MessageMenuAction>: View { | |
| } | ||
|
|
||
| /// If we're in landscape mode, adjust the `horizontalOffset` appropriately | ||
| if UIScreen.main.bounds.width > UIScreen.main.bounds.height { | ||
| if chatViewFrame.width > chatViewFrame.height { | ||
| switch alignment { | ||
| case .left: | ||
| horizontalOffset = UIApplication.safeArea.leading | ||
|
|
@@ -456,6 +469,7 @@ struct MessageMenu<MainButton: View, ActionEnum: MessageMenuAction>: View { | |
| if reactionSelectionIsVisible { | ||
| ReactionSelectionView( | ||
| viewModel: viewModel, | ||
| chatViewFrame: $chatViewFrame, | ||
| backgroundColor: theme.colors.messageFriendBG, | ||
| selectedColor: theme.colors.messageMyBG, | ||
| animation: .bouncy(duration: animationDuration), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it a binding here? you are not changing it inside ReactionSelectionView, right?