Skip to content

Rewrite File Handling #267

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

Merged
merged 23 commits into from
Mar 11, 2025
Merged

Rewrite File Handling #267

merged 23 commits into from
Mar 11, 2025

Conversation

pinkeshmars
Copy link
Collaborator

@pinkeshmars pinkeshmars commented Jan 30, 2025

Description

Rewrite File Handling

Linear ticket and magic word Fixes DEVR-380
Fixes DEVR-815
Fixes DEVR-817

Type of change

  • Typo fix
  • New feature
  • Enhancement to current docs
  • Removed outdated references
  • Update assets

@pinkeshmars pinkeshmars requested a review from PoojaB26 January 30, 2025 11:11
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@PoojaB26
Copy link
Collaborator

@pinkeshmars Should the name still be "file handling" though? I wonder if people would understand "media-management" right away because its not a super popular term? I think thats why we made the first version be called File Handling directly.

@pinkeshmars pinkeshmars changed the title Rewrite Media Management Rewrite File Handling Feb 11, 2025
@pinkeshmars
Copy link
Collaborator Author

@pinkeshmars Should the name still be "file handling" though? I wonder if people would understand "media-management" right away because its not a super popular term? I think thats why we made the first version be called File Handling directly.

@PoojaB26 Updated back to 'File Handling' everywhere.

Copy link
Collaborator

@PoojaB26 PoojaB26 left a comment

Choose a reason for hiding this comment

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

@pinkeshmars Added some comments and change requests. Apologies for the delay in reviewing it!

</div>
<p></p>

### Configure Upload or Save Media [Action]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not an action, also dont think we need a header for configurations, just use a simple text header instead of h3. title can be just "configurations"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about also removing "configurations" header? I have updated it so that it looks like a more natural flow. you can take a look and let me know.


Here’s an example of how you can use this action to upload media to your server via an API:

First, set the **Upload/Save Media** action with the **Local Upload (Widget State)** upload type. then, add the next action as an **API call** and select the API that will upload the file to your server. After the API call is complete, ensure your server returns the uploaded file's URL. Use this URL to save in the database or [display the uploaded image](displaying-media.md).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as before, less focus on the API part. You can add it as an additional example. But more focus on the bytes part and where all can we use the bytes object

  • one can be multipart APIs, constructing such an API (maybe a screenshot?)
  • and then calling API like your example,
  • then using the same local object to also upload to firebase/supabase so not just API,
  • maybe using the bytes for further processing like in custom code?

not all examples required but should show users all possible scenarios and then can show the above API example as one of the example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. added more example as you mentioned.

@PoojaB26
Copy link
Collaborator

PoojaB26 commented Mar 3, 2025

@pinkeshmars As you update this PR, can you also add a video embed for this updated video somewhere? https://www.youtube.com/watch?v=cKDuIZNdbh8

@pinkeshmars
Copy link
Collaborator Author

Hey @PoojaB26 all review comments are addressed! you can take a look again. Thanks!

Copy link
Collaborator

@PoojaB26 PoojaB26 left a comment

Choose a reason for hiding this comment

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

Please fix the small requests and then we can merge.


### Asset

You can also display media files uploaded to your [**Assets**](../../generated-code/directory-structure.md#assets). Assets are resources such as images, videos, documents, fonts, and other files that you include in your project. To upload assets, click on **Media Assets** in the left-side navigation menu and add files directly from your device. Alternatively, you can directly upload and display files when configuring media widgets by clicking the upload icon.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldnt reference assets generated code page here because the expectation is that this ref page would be the introduction to assets? rather add an admonition after this paragraph for people to see how assets are stored in generated code and then add the ref. but you should treat this section as THE introduction to assets so we shouldnt have to refer any other section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, updated.


:::tip[Generated Code]
The AudioPlayer uses the [**assets_audio_player**](https://pub.dev/packages/assets_audio_player) package for mobile and [**assets_audio_player_web**](https://pub.dev/packages/assets_audio_player_web) for web.
:::
Copy link
Collaborator

Choose a reason for hiding this comment

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

assets package already uses the web package so we dont think we need to mention that here, because it internally uses the web package already. correct me if i'm wrong!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, you are right! Actually after adding audio player widget to the project, I saw both packages got added to the pubspec file, so I added them directly.

- [**video_player**](https://pub.dev/packages/video_player): Provides core functionality for video playback from assets or network sources.
- [**video_player_android**](https://pub.dev/packages/video_player_android): Ensures smooth playback specifically for Android devices.
- [**video_player_avfoundation**](https://pub.dev/packages/video_player_avfoundation): Integrates with Apple's AVFoundation framework for playback on iOS devices.
- [**video_player_web**](https://pub.dev/packages/video_player_web): Enables video playback support for web browsers.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as before, it uses these packages internally, so we only need to mention the main package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ya, no problem.

@pinkeshmars pinkeshmars merged commit 3f7c800 into main Mar 11, 2025
1 check passed
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