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

Improvements to file/image upload handling UX #4578

Merged
merged 11 commits into from
Oct 1, 2023
Merged

Conversation

ssddanbrown
Copy link
Member

@ssddanbrown ssddanbrown commented Sep 25, 2023

Related to #4454.

  • Adds friendlier handling, with proper message, for requests caught by Laravel's ValidatePostSize middleware.
  • Refactors and splits out much of the image file handling.

Todo

  • Roll out OOM handling to other endpoints where thumbs are created
    • Roll out to gallery responses, so images are created via these means but the original view is still returned if an OOM occurance happens. Needs juggling of view and thumbnail gen.
      • Drawings
      • Images
  • Test new thumbnail regen endpoint
  • Fix large thumbnails when image manager only has single/few images in list.

Notes

  • See Illuminate\Foundation\Bootstrap\HandleExceptions for example of reserving memory and handling errors (via register_shutdown_function). Can run custom hook through register_shutdown_function. Need to test conflicts with Laravel's own handling.
  • Need to consider for API also.

Screenshots

Preview of added image manager handling:

Screenshot 2023-10-01 at 12-54-46 Editing Page Comment testing The BookStack

Uploads over the post max size Would previously error without a
clean user facing message. This catches that error to provide a
user friendly message, compatible with our common error handling.

Tested on image manager handling.
Added test to cover.
- Added some level of app out-of-memory handling so we can show a proper
  error message upon OOM events.
- Added endpoint and image-manager button/action for regenerating
  thumbnails for an image so they can be re-created upon failure.
Added since we can't always be sure of future image usage, and in many
cases we don't generate ahead-of-time.
Also:
- Simplified image handling on certain models.
- Updated various string handling operations to use newer functions.
To break it up.
Also added better memory handling to other parts of the app.
Extracts duplicated required handling (Like path adjustment) out to
simpler storage disk instance which can be passed around.
- Moved thumnbail loading out of repo into ImageResizer.
- Updated gallery and editor image handling to show errors where
  possible to indicate memory issues for resizing/thumbs.
- Updated gallery to load image data in a per-image basis via edit form
  for more resiliant thumb/data fetching. Data was previously provided
  via gallery listing, which could be affected by failing generation
  of other images.
- Updated image manager double click handling to be more pleasant and
  not flash away the edit form.
- Updated editor handlers to use main URL when thumbs fail to load.
@ssddanbrown ssddanbrown merged commit 8bba5dd into development Oct 1, 2023
@ssddanbrown ssddanbrown deleted the upload_handling branch October 1, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant