Skip to content

Revert "feat(core): add app-region: drag to data-tauri-drag-region (#9789)"#9860

Merged
lucasfernog merged 4 commits into
devfrom
revert-9789
May 29, 2024
Merged

Revert "feat(core): add app-region: drag to data-tauri-drag-region (#9789)"#9860
lucasfernog merged 4 commits into
devfrom
revert-9789

Conversation

@amrbashir
Copy link
Copy Markdown
Member

@amrbashir amrbashir commented May 23, 2024

This reverts commit ae6b13d.

While the app-region: drag does work correctly and fix the touch issue, it has a few differences:

  • Doesn't allow Right Click, as it will always show the system context menu
  • data-tauri-drag-region works only if the click was on an element that has it, this allows buttons in the custom titlebar to work, however app-region: drag will treat the whole area as a titlebar won't even allow clicks on buttons

The buttons issue could be solved by users if they structure their titlebar to separate the buttons container from the container that has app-region: drag, like this:

<div class="grid-with-right-and-left-sections">
  <div class="grid-left-section app-drag-region">Window title</div>
  <div class="grid-right-section">
    <button> Minimize </button> 
    <button> Maximize </button>
    <button> Close </button>
  </div>
</div>

But realistically speaking, this will become harder when implementing a complex titlebar like VSCode custom titlebar implementation, and app-region: drag doesn't provide the needed flexibility.

In conclusion, I think we should revert this and just document that users can add app-region: drag if they wish so.

@amrbashir amrbashir requested a review from a team as a code owner May 23, 2024 18:55
FabianLars
FabianLars previously approved these changes May 23, 2024
Copy link
Copy Markdown
Member

@FabianLars FabianLars left a comment

Choose a reason for hiding this comment

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

rip

@Legend-Master
Copy link
Copy Markdown
Contributor

Legend-Master commented May 24, 2024

To be honest, vscode's title bar is implemented using app-region: drag, it's more about data-tauri-drag-region being not flexible enough (we don't have a way to add exclude zones), and about the menu thing, vscode uses some additional code to do that

microsoft/vscode#155276
https://github.com/microsoft/vscode/blob/e82e806f172f30aecb0e6bb04c01f37780c16f22/src/vs/platform/windows/electron-main/windowImpl.ts#L149-L193

I think we should go with the other way around, document this and let users to opt-out (they can just do app-region: no-drag) (or we add in a data-tauri-non-drag-region or something)

@amrbashir
Copy link
Copy Markdown
Member Author

amrbashir commented May 24, 2024

To be honest, vscode's title bar is implemented using app-region: drag

yes but it is cross-platfrom because they are using Chrome which is not the case for us,

I think we should go with the other way around, document this and let users to opt-out (they can just do app-region: no-drag) (or we add in a data-tauri-non-drag-region or something)

app-region: drag breaks consistency and compatibility of this feature between the different platforms we have and is why I want to revert this.

However the fact that app-region: drag allows dragging with touch, is a huge win for me and why I don't want to merge this yet.

I just want to discuss and find an acceptable middle ground, maybe we change the data-tauri-drag-region implementation to act like windows on all platforms, maybe we just document the changes. I want to hear what others think

@Borber
Copy link
Copy Markdown

Borber commented May 29, 2024

I also hope to revert this commit. I think we haven't designed this yet. This update basically completely destroyed the software I wrote Tran. It originally had the entire window draggable, and you can directly interact with the elements below by double-clicking, and you can also scroll, and the menu will not appear when you right-click. I thought about it for a long time last night and couldn't figure out how to do this elegantly under the current solution.

@Legend-Master
Copy link
Copy Markdown
Contributor

Legend-Master commented May 29, 2024

You can use *[data-tauri-drag-region] { app-region: unset; } for now to get the old behavior back

I'm still leaning towards using as much native things as possible (probably because I got hurt way too many times on bad custom ones), but those are all valid points on why this should be opt-in not opt-out

Guess we can document this and making people opt-in if they only plan to use it as a normal title bar

@Borber
Copy link
Copy Markdown

Borber commented May 29, 2024

You can use *[data-tauri-drag-region] { app-region: unset; } for now to get the old behavior back

I'm still leaning towards using as much native things as possible (probably because I got hurt way too many times on bad custom ones), but those are all valid points on why this should be opt-in not opt-out

Guess we can document this and making people opt-in if they only plan to use it as a normal title bar

I encountered a problem. I directly set its style or drag as you said. When I set the force, it cannot be dragged.

image

@amrbashir
Copy link
Copy Markdown
Member Author

We should merge this asap and release @lucasfernog , then we can iterate on a better solution in another PR

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.

[bug] Tauri v2, the child elements of 'data-tauri-drag-region' cannot trigger events

5 participants