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

feat: import pdf from home menu #869

Merged
merged 6 commits into from
Aug 8, 2023
Merged

Conversation

ZebraVogel94349
Copy link
Contributor

@ZebraVogel94349 ZebraVogel94349 commented Aug 7, 2023

This makes it possible to import PDF files when selecting the "import note" option in the home menu and then selecting a PDF file in the file picker. This should also fix #771 by setting the "withData" parameter in the file picker to false.

@codecov

This comment was marked as off-topic.

@adil192
Copy link
Member

adil192 commented Aug 8, 2023

I don't think this would solve #771 since the full content of the PDF is still loaded into memory.
Besides that, thank you again for your hard work!

@adil192 adil192 merged commit ab47185 into saber-notes:main Aug 8, 2023
@ZebraVogel94349
Copy link
Contributor Author

ZebraVogel94349 commented Aug 9, 2023

I don't think this would solve #771 since the full content of the PDF is still loaded into memory.

When I tried importing large .sbn2 files with the parameter set to true, the app always crashed on my tablet. With the parameter set to false, it didn't crash anymore. According to miguelpruivo/flutter_file_picker#392 (comment) the files are not loaded into memory directly after picking them, but only cached, if withData is set to false. The content is still loaded into memory, but I think, the problem is not a lack of memory to store the content of a file but the FilePicker Library trying to allocate the memory to quickly/without checking if there is enough memory available.

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