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

Rewrite composer image state management, remove MobX #3925

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

pfrazee
Copy link
Collaborator

@pfrazee pfrazee commented May 9, 2024

Supersedes #3890 by @mary-ext.


Original PR description:

Decided to do this separately from #3813, removes the only remaining MobX dependency from the app.

  • Temporary image files are moved to {cacheDirectory}/bsky-composer, this folder is set to purge when the composer is closed. I feel this is a worthwhile approach compared to making sure that images are being purged properly every time it's being manipulated.
  • As a side benefit, the new image models are immutable.
  • ALF'd image alt text and edit dialog
Alt text dialog Edit image dialog
  • Working with image cropping libraries is just as wonderful as I remembered it to be, react-avatar-editor has been replaced with react-image-crop which is slightly more workable. Would've preferred one that also does fixed cropping but react-easy-crop is being finicky with initial states.
  • The image cropper dialog for avatars and banners still exists, but it's been cleaned up slightly

Additions

  • Removed aspect ratio controls from the composer image editor. Not really needed now that there's a cropping box, and it wasn't working correctly.
  • Fixes to the cropping modal used by UserAvatar and UserBanner

Copy link

render bot commented May 9, 2024

Copy link

github-actions bot commented May 9, 2024

The Pull Request introduced fingerprint changes against the base commit: 55ac287

Fingerprint diff
[{"type":"file","filePath":"package.json","reasons":["expoConfigPlugins"],"hash":"5bfc50879ff239e0c12cf79f3eed104b39baf3ec"}]

Generated by PR labeler 🤖

Copy link

github-actions bot commented May 9, 2024

Old size New size Diff
6.44 MB 6.44 MB 157 B (0.00%)

@mary-ext
Copy link
Contributor

mary-ext commented May 9, 2024

The new modals might make it easy to accidentally lose work if one manages to overshoot their mouse cursor while moving/resizing the crop area around.

I'll have to update #3884 to get it to work for the new modals as well if we still want to close on backdrop click but not on the case I just mentioned

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.

None yet

2 participants