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

Always show back button on mac catalyst #1225

Conversation

matthewrfennell
Copy link
Member

#1205 introduced a regression on MacOS where, once a user opens the contact details view, they cannot close it, since there is no back button and clicking outside of the modal does nothing (unlike on iOS).

This PR works around this by always showing the back button in AddTopLevelNavigation. The consequence of this is that we also show the back button in popovers (such as the ContactsView popover)

Screenshot 2024-08-31 at 15 59 53 Screenshot 2024-08-31 at 16 00 06

matthewrfennell and others added 2 commits August 31, 2024 16:11
This is because, whether we should display the back button is not
necessarily dependent on the particular size class.
On iOS, you can dismiss modals by dragging down on them, or by tapping
outside of the modal. The same affordances are not made on MacOS. This
means that, if you open the contact details view on catalyst at the
moment, it is impossible to close the view, since we have disabled
showing the back button in AddTopLevelNavigation when the size class is
regular (which is normally the case on MacOS).

Therefore, if the current target is mac catalyst, we should always
display back buttons on modals, so that users can dismiss them without
having to force close the app.

Thanks lissine for finding this, and suggesting how to fix it.

Co-authored-by: lissine <[email protected]>
@tmolitor-stud-tu
Copy link
Member

Wouldn't it be possible to use #if targetEnvironment(macCatalyst) to only add the back button on macos but not on ios (iphone/ipad)?

@tmolitor-stud-tu
Copy link
Member

Okay, forget my comment. You exactly did what I had in mind :)

@tmolitor-stud-tu tmolitor-stud-tu merged commit 009becf into monal-im:develop Sep 1, 2024
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.

2 participants