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

#2208 Add interactive dragging to .anytypeSheet #2221

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

stansidel
Copy link


  • I understand that contributing to this repository will require me to agree with the CLA

Description

This PR adds interactive dragging capability to .anytypeSheet used for popup like presentation (object settings for example). It should fix #2208.

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI

Related Tickets & Documents

#2208

Mobile & Desktop Screenshots/Recordings

Screen.Recording.2024-10-15.at.7.36.48.AM.mov

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help

Added to documentation?

  • 📜 README.md
  • 📓 tech-docs
  • 🙅 no documentation needed

@stansidel stansidel requested a review from a team as a code owner October 15, 2024 02:55
Copy link

github-actions bot commented Oct 15, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@stansidel stansidel force-pushed the issue-2208-anytypesheet-dragging branch from e23eeae to 74043a5 Compare October 15, 2024 03:00
@stansidel stansidel force-pushed the issue-2208-anytypesheet-dragging branch from 74043a5 to f22d931 Compare October 15, 2024 03:02
@stansidel
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@mgolovko
Copy link
Collaborator

@stansidel thank you for your help!

There are some issues that need to be improved:

  • AnytypeSheet doesn't have to add DragIndicator. The screen adds it if necessary.
  • The screen size should not change.
  • The keyboard is not handled.
  • We expect that dragging will work anywhere on the screen and will not conflict with horizontal scrolls inside the screen.
Case.1.mov
Case.2.mov

@ignatovv
Copy link
Member

ignatovv commented Oct 15, 2024

I have updated the issue with richer details from @mgolovko comment
image

@stansidel
Copy link
Author

@mgolovko Thank you for reviewing my PR and making suggestions. I'll make sure to work on the improvements soon.

@stansidel
Copy link
Author

I've made the following changes to the PR:

  • Removed DragIndicator from anytypeSheet.
  • Added DragIndicator to ObjectSettingsView.
  • Fixed an issue where the bottom sheet height could change while dragging in certain cases.

I would appreciate more clarification regarding the two remaining improvements:

  • Keyboard Handling: Could you provide more details on the expected behavior when the keyboard is visible?
  • Drag Behavior: We expect that dragging should work anywhere on the screen without conflicting with horizontal scrolls inside the sheet. Could you explain how you envision this interaction?

To assist with comparison, I’ve recorded and attached the following:

anytypeSheet vs. Default SwiftUI sheet Dragging Behavior:

Recording with scrollable content inside the bottom sheet using anytypeSheet:

scrolling-fullscreen.mov

Recording with scrollable content inside the bottom sheet using the default SwiftUI sheet:

scrolling-sheet.mov

Dragging with Keyboard Visible:

Recording using anytypeSheet:

keyboard-fullscreen.mov

Recording using the default SwiftUI sheet:

keyboard-sheet.mov

@mgolovko, could you kindly describe your expectations for the bottom sheet’s behavior in these two scenarios?

@joe-pusya
Copy link
Member

joe-pusya commented Oct 23, 2024

Hey @stansidel thank you for your work! I just had a look on your implementation/comments and would like to answer on it:

About drag gesture - we would like to avoid clashing between any other gesture (taps, scrolls (ideally scrollview too) and other).
What I mean by examples:

Our anytypeSheet implementation problems:

  1. When you finger goes down and then up - sheet closes, you should understand the direction of dragging
  2. When you clash with another gesture (tap for example) anytypeSheet becomes non-interactive and just closes at the end
  3. Scrollview inside anytypeSheet -> interaction doesn't work I think so
    https://github.com/user-attachments/assets/9af4e7c6-2ca5-4bc6-8c69-35e0b0182f3e

SwiftUI sheet behaviour to compare (I didn't understand your examples with it, can you please explain what do you mean?)
https://github.com/user-attachments/assets/8f627a3c-3354-4b11-8182-2ecac4ab3c3f

@stansidel
Copy link
Author

Hi @joe-pusya,

Thank you for your comments. I guess I used the native sheet incorrectly in my examples because, in my implementation, there was a visual effect of the views in the background getting narrower.

As I see in your example of the native behavior here https://github.com/user-attachments/assets/8f627a3c-3354-4b11-8182-2ecac4ab3c3f, it works just like you'd expect the custom implementation to work. Could you please explain then what are the goals of using a custom implementation of the sheets over the native one as demonstrated in the video you provided?

@joe-pusya
Copy link
Member

joe-pusya commented Oct 23, 2024

Hi @joe-pusya,

Thank you for your comments. I guess I used the native sheet incorrectly in my examples because, in my implementation, there was a visual effect of the views in the background getting narrower.

As I see in your example of the native behavior here https://github.com/user-attachments/assets/8f627a3c-3354-4b11-8182-2ecac4ab3c3f, it works just like you'd expect the custom implementation to work. Could you please explain then what are the goals of using a custom implementation of the sheets over the native one as demonstrated in the video you provided?

It's just because of design request) The main goal is to implement floating sheet (clear space under the content, proper design like round corner and not stacking behind screens) with native swiftui sheet capabilities

@ignatovv ignatovv added the WIP label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current implementation of anytypeSheet have no interactive drag capabilities
4 participants