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

Support open/save dialogs on wayland. #2228

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

jneem
Copy link
Collaborator

@jneem jneem commented Jul 19, 2022

There's one little to-do left: the parenting isn't right because the window-identifier business in ashpd isn't working right now because of wayland crate version mismatches.

@@ -369,6 +371,14 @@ impl Handle for Surface {
return self.inner.wl_surface.borrow().invalidate_rect(rect);
}

fn open_file(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor question: do we need to add these to every surface or can we just use the already implemented data() method to access the implementations directly vs expanding the api surface of handle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was kinda wondering that too, but it seemed better to just follow what all the other methods were doing... I'd be happy to change it, though

Copy link
Contributor

Choose a reason for hiding this comment

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

the PR is fine strictly speaking as is; since you're basically following the pattern every backend is doing.... its just a nit i have with druids internals. these methods shouldn't need to be nested as deeply into the backends as they are. a backend really should not care about opening file dialogs. i've covered the topic in detail in other issues.

looks like you're only using the idle handle. which is already exposed, and long term the wayland id. I'd try to keep the file stuff as lightly coupled as possible personally. but rather see functionality implemented than worry about architectural issues that impact every backend currently =)

@@ -469,6 +478,26 @@ impl Data {
self.schedule_deferred_task(DeferredTask::Paint);
}

fn open_file(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
// FIXME: current ashpd has issues with wayland versions
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a link to the upstream issue we could put here as a reference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess this. Which is merged, but there's still an issue with asphd'd wayland-client version not matching ours (and depending on a pre-release, which is a bit of a pain). I think the sanest thing to do is to wait until version 0.3 of the wayland-client libraries.

Also, it looks like getting an identifier requires a round-trip to the compositor, and so the new function is async...

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