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

X11 dialogs, take 2. #2153

Merged
merged 5 commits into from
Apr 22, 2022
Merged

X11 dialogs, take 2. #2153

merged 5 commits into from
Apr 22, 2022

Conversation

jneem
Copy link
Collaborator

@jneem jneem commented Mar 17, 2022

This supersedes #1774 (which I'll close). It has the same bug, which doesn't seem solvable without some acknowledgement from xdg-desktop-portal. I'm still not thrilled about this situation, but I don't see another viable path forward to native file dialogs on x11 (and wayland). Therefore I'm inclined to just merge this despite the issue.

if let Err(e) = block_on(async {
let conn = zbus::Connection::session().await?;
let proxy = file_chooser::FileChooserProxy::new(&conn).await?;
// TODO: use the window id. This requires changes in ashpd.

Choose a reason for hiding this comment

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

Yes that is something I want to change, by splitting things between x11 and the Wayland implementation.

Could you open an issue for that and I will have a look at doing that this weekend ?

Choose a reason for hiding this comment

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

Unrelated but doesn't the crate supports passing a window handle under Wayland as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Eventually yes, but this is just in our x11 backend for now.

druid-shell/src/backend/x11/dialog.rs Show resolved Hide resolved
Copy link
Collaborator

@maan2003 maan2003 left a comment

Choose a reason for hiding this comment

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

LGTM! Just a minor nit.

Comment on lines 91 to 95
if open {
callback(proxy.open_file(&id, title, options.into()).await?.uris());
} else {
callback(proxy.save_file(&id, title, options.into()).await?.uris());
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason to use a callback? can't we just let uris = if open ..... and then inline callback

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, the result of uris() is borrowed from the objects returned from open_file or save_file, so that version will drop the temporary objects too soon. But it works with some extra temporary variables, and indeed I think it's nicer. Thanks!

This doesn't actually have any effect (because ashpd doesn't support
XcbHandle yet) and it brings in unnecessary dependencies.
@jneem jneem merged commit 03f63e7 into linebender:master Apr 22, 2022
raphlinus pushed a commit that referenced this pull request May 4, 2022
Use xdg-desktop-portal's dbus APIs for open/save dialogs on x11.
xarvic pushed a commit to xarvic/druid that referenced this pull request Jul 29, 2022
Use xdg-desktop-portal's dbus APIs for open/save dialogs on x11.
xarvic pushed a commit to xarvic/druid that referenced this pull request Jul 29, 2022
Use xdg-desktop-portal's dbus APIs for open/save dialogs on x11.
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.

3 participants