Skip to content

Attachment upload with file dialog#674

Open
alanpoon wants to merge 37 commits intoproject-robius:mainfrom
alanpoon:image_upload
Open

Attachment upload with file dialog#674
alanpoon wants to merge 37 commits intoproject-robius:mainfrom
alanpoon:image_upload

Conversation

@alanpoon
Copy link
Copy Markdown
Contributor

Screen.Recording.2026-01-26.at.4.09.11.PM.mov
  • macOS
  • Android
  • Linux
  • Windows

@alanpoon alanpoon self-assigned this Jan 26, 2026
@kevinaboos
Copy link
Copy Markdown
Member

awesome!

I know this isn't yet complete, but I wanted to quickly drop in and suggest using rfd instead of makepad's internal file API. rfd is the de facto crate for this kind of thing, and I plan to shortly create a robius crate that wraps rfd, so we'll end up with a similar API.

Plus, with rfd you'd get immediate support for all 3 desktop platforms.

@alanpoon
Copy link
Copy Markdown
Contributor Author

awesome!

I know this isn't yet complete, but I wanted to quickly drop in and suggest using rfd instead of makepad's internal file API. rfd is the de facto crate for this kind of thing, and I plan to shortly create a robius crate that wraps rfd, so we'll end up with a similar API.

Plus, with rfd you'd get immediate support for all 3 desktop platforms.

Kindly refer to this implementation of the rfd. https://github.com/Vjze/YY_DPS/blob/edb15ffc85b646c27547081b30a0e6f0d8ba688b/src/export/export_view.rs#L158 This requires the tokio runtime in a field in export screen

@kevinaboos
Copy link
Copy Markdown
Member

awesome!
I know this isn't yet complete, but I wanted to quickly drop in and suggest using rfd instead of makepad's internal file API. rfd is the de facto crate for this kind of thing, and I plan to shortly create a robius crate that wraps rfd, so we'll end up with a similar API.
Plus, with rfd you'd get immediate support for all 3 desktop platforms.

Kindly refer to this implementation of the rfd. https://github.com/Vjze/YY_DPS/blob/edb15ffc85b646c27547081b30a0e6f0d8ba688b/src/export/export_view.rs#L158 This requires the tokio runtime in a field in export screen

Ok, what's the issue with that? Is that problematic? Apologies, but I'm not quite sure what point you're trying to make.

Moly has used rfd in their app, works fine AFAIK.

@alanpoon alanpoon added the waiting-on-review This issue is waiting to be reviewed label Jan 30, 2026
@alanpoon alanpoon marked this pull request as ready for review January 30, 2026 16:18
@alanpoon
Copy link
Copy Markdown
Contributor Author

awesome!
I know this isn't yet complete, but I wanted to quickly drop in and suggest using rfd instead of makepad's internal file API. rfd is the de facto crate for this kind of thing, and I plan to shortly create a robius crate that wraps rfd, so we'll end up with a similar API.
Plus, with rfd you'd get immediate support for all 3 desktop platforms.

Kindly refer to this implementation of the rfd. https://github.com/Vjze/YY_DPS/blob/edb15ffc85b646c27547081b30a0e6f0d8ba688b/src/export/export_view.rs#L158 This requires the tokio runtime in a field in export screen

Ok, what's the issue with that? Is that problematic? Apologies, but I'm not quite sure what point you're trying to make.

Moly has used rfd in their app, works fine AFAIK.

Thanks for referring me to Moly. File Dialog does not work well in asynchronously in macOS as file dialog is only allowing in main thread.

Copy link
Copy Markdown
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Thanks Alan, looks very good — impressive work here!

I left comments about a few major structural decisions, but it's mostly just about refactoring things into more modular widgets. I also left some questions about using higher-level Timeline APIs vs Room APIs for sending attachments.

@kevinaboos kevinaboos removed the waiting-on-review This issue is waiting to be reviewed label Feb 5, 2026
@kevinaboos kevinaboos assigned kevinaboos and alanpoon and unassigned alanpoon Feb 5, 2026
@kevinaboos kevinaboos added the waiting-on-author This issue is waiting on the original author for a response label Feb 5, 2026
@kevinaboos
Copy link
Copy Markdown
Member

  • macOS
  • Android
  • Linux
  • Windows

Also, now that you're using rfd, can you confirm whether this also works for Linux and Windows? Theoretically they are also included as supported platforms in rfd.

@alanpoon alanpoon added waiting-on-review This issue is waiting to be reviewed and removed waiting-on-author This issue is waiting on the original author for a response labels Feb 25, 2026
Copy link
Copy Markdown
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Looks pretty good, just a few minor comments.

@kevinaboos kevinaboos added waiting-on-author This issue is waiting on the original author for a response and removed waiting-on-review This issue is waiting to be reviewed labels Feb 25, 2026
alanpoon and others added 8 commits February 26, 2026 08:25
Co-authored-by: Kevin Boos <1139460+kevinaboos@users.noreply.github.com>
Co-authored-by: Kevin Boos <1139460+kevinaboos@users.noreply.github.com>
Co-authored-by: Kevin Boos <1139460+kevinaboos@users.noreply.github.com>
@alanpoon alanpoon added waiting-on-review This issue is waiting to be reviewed and removed waiting-on-author This issue is waiting on the original author for a response labels Feb 26, 2026
Copy link
Copy Markdown
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes.

I see that you ran an auto-formatter on the code, which makes it impossible for me to review the key changes that you actually made. Take a look at the room_input_bar diff, for example, and you'll see just how many unnecessary changes were made.

A friendly reminder to not run tools like rustfmt on whole files because it destroys our existing custom formatting decisions. Please undo all unnecessary formatting-only changes across all changed files (most of which are not an improvement, particularly in the room_input_bar.rs file), and then manually inspect your own PR's diff to ensure that there are no superfluous changes. Then, after that, please re-request a review from me.

@kevinaboos kevinaboos added waiting-on-author This issue is waiting on the original author for a response and removed waiting-on-review This issue is waiting to be reviewed labels Mar 14, 2026
@kevinaboos
Copy link
Copy Markdown
Member

I've just merged the migration to Makepad 2.0. For this PR, kindly stick with Makepad 1.0 and fully address my comments here, and then once we have agreed, then we can start the migration to 2.0. I'm happy to help as well, if there are any parts of 2.0 that you're unsure of.

@alanpoon alanpoon removed the waiting-on-author This issue is waiting on the original author for a response label Mar 23, 2026
Copy link
Copy Markdown
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Thanks, everything looks good! Just one small comment, and then you can proceed with the migration to Makepad 2.0.

{
log!("Retrying upload");
// Hide retry button
self.button(ids!(retry_button)).set_visible(cx, false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: instead of hiding buttons, which is typically a bad user experience, it's better to just disable them. (And don't forget to re-enable them when needed).

That also avoids the problem of the UI jumping around/moving unexpectedly.

@alanpoon alanpoon mentioned this pull request Mar 26, 2026
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