Attachment upload with file dialog#674
Attachment upload with file dialog#674alanpoon wants to merge 32 commits intoproject-robius:mainfrom
Conversation
|
awesome! I know this isn't yet complete, but I wanted to quickly drop in and suggest using Plus, with |
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 |
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. |
eae0bfa to
f922cf2
Compare
kevinaboos
left a comment
There was a problem hiding this comment.
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.
Also, now that you're using |
e78c41b to
81f4718
Compare
Co-authored-by: Kevin Boos <1139460+kevinaboos@users.noreply.github.com>
Co-authored-by: Kevin Boos <1139460+kevinaboos@users.noreply.github.com>
kevinaboos
left a comment
There was a problem hiding this comment.
Looks pretty good, just a few minor comments.
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>
kevinaboos
left a comment
There was a problem hiding this comment.
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.
| /// Whether the view is showing an error state with retry option | ||
| #[rust] | ||
| is_error_state: bool, | ||
| /// File data to retry if in error state | ||
| #[rust] | ||
| retry_file_data: Option<FileData>, |
There was a problem hiding this comment.
why do you need both an Option and a boolean, isn't that redundant?
If you do need both, then it's much better and clearer to create a single enum that distinguishes between the 3 or 4 different possible cases, and only includes the FileData in the variant(s) that it is needed for.
Screen.Recording.2026-01-26.at.4.09.11.PM.mov