Skip to content

fix: Edited changes are saved on orientation change. #2790

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 26, 2019
Merged

fix: Edited changes are saved on orientation change. #2790

merged 1 commit into from
Jun 26, 2019

Conversation

@yashk2000
Copy link
Member

@yashk2000 yashk2000 commented Jun 24, 2019 •
edited
Loading

Fixed #2702
Fixed #2789

Changes: All edits in an image are stored when orientation change happens.

GIF:

ezgif com-video-to-gif (2)

@auto-label auto-label bot added the fix label Jun 24, 2019
@yashk2000
Copy link
Member Author

@iamareebjamal @abishekvashok @mariobehling please review.

@yashk2000
Copy link
Member Author

@adityastic please review.

@mariobehling
Copy link
Member

mariobehling commented Jun 24, 2019 •
edited
Loading

Please provide a gif that is slower and a gif that does not use pictures of another app as example. Phimpme is a photo app mainly. Could you please use photos to avoid confusion of users by using screenshots of apps to showcase the app?

@yashk2000
Copy link
Member Author

@mariobehling sure. Just a minute.

@yashk2000
Copy link
Member Author

@mariobehling gif updated.

adityastic
adityastic suggested changes Jun 25, 2019
View reviewed changes
app/src/main/java/org/fossasia/phimpme/editor/fragment/CropFragment.java Outdated
Copy link

Choose a reason for hiding this comment

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

Dont. This is a memory leak

app/src/main/java/org/fossasia/phimpme/editor/EditImageActivity.java Outdated
Copy link

Choose a reason for hiding this comment

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

dont make anything static, save this also in the savedState

app/src/main/java/org/fossasia/phimpme/editor/fragment/AddTextFragment.java Outdated
Copy link

Choose a reason for hiding this comment

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

same, static variable will not be garbage collected

app/src/main/java/org/fossasia/phimpme/editor/fragment/CropFragment.java Outdated
Copy link

Choose a reason for hiding this comment

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

same

app/src/main/java/org/fossasia/phimpme/editor/fragment/CropFragment.java Outdated
Copy link

Choose a reason for hiding this comment

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

even here xD

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll leave this one. This one wasn't done by me. It's been there from the beginning.

Copy link

Choose a reason for hiding this comment

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

Ok, No worries

app/src/main/java/org/fossasia/phimpme/editor/fragment/SliderFragment.java Outdated
Copy link

Choose a reason for hiding this comment

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

same

app/src/main/java/org/fossasia/phimpme/editor/view/CustomPaintView.java Outdated
Copy link

Choose a reason for hiding this comment

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

.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cannot avoid this one. I need to use the variable in other classes to retain state.

Copy link

@adityastic adityastic Jun 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

you can retain the variable in this view class itself

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried that. The app crashed. When I reverted, it worked.

app/src/main/java/org/fossasia/phimpme/editor/view/RotateImageView.java Outdated
Copy link

Choose a reason for hiding this comment

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

.

app/src/main/java/org/fossasia/phimpme/editor/view/RotateImageView.java Outdated
Copy link

Choose a reason for hiding this comment

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

.

app/src/main/java/org/fossasia/phimpme/editor/view/StickerView.java Outdated
Copy link

Choose a reason for hiding this comment

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

.

@yashk2000
Copy link
Member Author

@adityastic done. I've only kept static in places where it was really needed. Rest have been removed.

@yashk2000
Copy link
Member Author

@abishekvashok @iamareebjamal @mariobehling please review and merge.

@yashk2000
Copy link
Member Author

@abishekvashok @iamareebjamal @mariobehling please merge.

@abishekvashok abishekvashok merged commit 56b87d2 into fossasia:development Jun 26, 2019
@abishekvashok
Copy link
Member

Thanks! Great job!

This was referenced Jun 26, 2019
Closed
Closed
Closed
pull bot pushed a commit to sahilsaha7773/phimpme-android that referenced this pull request Jul 18, 2019
@abishekvashok
Merge pull request fossasia#2790 from yashk2000/rot_lock
09bc539 
fix: Edited changes are saved on orientation change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vinaysajjanapu vinaysajjanapu Awaiting requested review from vinaysajjanapu

@Subhankar29 Subhankar29 Awaiting requested review from Subhankar29

2 more reviewers

@abishekvashok abishekvashok abishekvashok approved these changes

@adityastic adityastic adityastic approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Edits in image not saved on orientation change. Content of stickers removed when changing the orientation

4 participants

@yashk2000 @mariobehling @abishekvashok @adityastic