Skip to content

Fix the MessageMenu displaying elements outside the window after resizing it#278

Open
lissine0 wants to merge 1 commit into
exyte:mainfrom
lissine0:fix-241
Open

Fix the MessageMenu displaying elements outside the window after resizing it#278
lissine0 wants to merge 1 commit into
exyte:mainfrom
lissine0:fix-241

Conversation

@lissine0

Copy link
Copy Markdown
Contributor

UIScreen.main.bounds returns the physical screen size. It shouldn't be used for layout calculations because it's possible to resize the window on iPads.

Fixes #241

Before:

before.mp4

After:

after.mp4

…zing it

UIScreen.main.bounds returns the physical screen size.
It shouldn't be used for layout calculations because it's possible to resize
the window on iPads.

Fixes exyte#241
Rectangle()
.fill(.primary.opacity(0.1))
}
.edgesIgnoringSafeArea(.all)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

wrong alignment here, please make sure the formatting is neat

.maxHeightGetter($reactionOverviewHeight)
.offset(y: UIApplication.safeArea.top)
.transition(defaultTransition)
GeometryReader { geometry in

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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?

@State private var xOffset: CGFloat = 0.0
@State private var yOffset: CGFloat = 0.0
@State private var viewState: ViewState = .initial
@Binding var chatViewFrame: CGRect

Copy link
Copy Markdown
Collaborator

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?

.frameGetter($messageMenuFrame)
.position(x: chatViewFrame.width / 2 + horizontalOffset, y: verticalOffset)
.opacity(messageMenuOpacity)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please remove the extra \n


/// Overall ChatView Frame
let chatViewFrame: CGRect = UIScreen.main.bounds
@State private var chatViewFrame: CGRect = .zero

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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?

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.

macOS: message interaction elements like copy, delete and emoji reactions should always be visible inside the window

2 participants