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

fix: respect presetRatio on new cropzone #700

Merged

Conversation

adzlocal
Copy link
Contributor

Please check if the PR fulfills these requirements

  • It's submitted to right branch according to our branching model
  • It's right issue type on title
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • It does not introduce a breaking change or has description for the breaking change

Description

Problem (how it is now):

  • in crop mode
  • choose a preset ratio (eg. 16/9)
  • draw/make a new cropzone
  • the aspect ratio is not restricted to the chosen preset ratio (not like when resizing the cropzone by the corners)

This PR fixes that problem. When you now draw/make a new cropzone while having selected a preset ratio, that preset ratio is now respected.

@lja1018
Copy link
Contributor

lja1018 commented Feb 28, 2022

@adzlocal
Excuse me for not having answered your PR sooner.
Thank you for your contribution. I'll review it later.

@adzlocal
Copy link
Contributor Author

@lja1018 When do you think will you be able to review the PR? We would love to have this issue fixed upstream.

@lja1018
Copy link
Contributor

lja1018 commented Mar 30, 2022

@adzlocal
Sorry, I'm late. I will review it today.

@lja1018
Copy link
Contributor

lja1018 commented Mar 30, 2022

@adzlocal
Thank you again for your contribution and merge this PR.

@lja1018 lja1018 merged commit 418e5fe into nhn:master Mar 30, 2022
lja1018 added a commit that referenced this pull request Mar 30, 2022
@lja1018
Copy link
Contributor

lja1018 commented Mar 30, 2022

I misclicked 'revert'.

@adzlocal
Copy link
Contributor Author

adzlocal commented Apr 6, 2022

@lja1018 Thank you for the review! Could you please update the npm package as well?
https://www.npmjs.com/package/@toast-ui/vue-image-editor

@lja1018
Copy link
Contributor

lja1018 commented Apr 13, 2022

@adzlocal
Thank you. :)
I'll let you know when I deploy the package.

@adzlocal
Copy link
Contributor Author

@lja1018 Hey, the package still hasn't received an update – would you be able to do it till end of May? Thanks!

@lja1018
Copy link
Contributor

lja1018 commented May 18, 2022

@adzlocal
reflected in v3.15.3. Do you want to check?

@adzlocal
Copy link
Contributor Author

@lja1018 We tried to update to v3.15.3 recently but this version disappeared from the npm package repository – could you please publish that version (again)? Thank you!

@adzlocal
Copy link
Contributor Author

adzlocal commented Sep 9, 2022

@lja1018 There's still no v3.15.3 available in the npm repository – as this fix should have been available since April could you please have another look at this issue?

@lja1018
Copy link
Contributor

lja1018 commented Sep 14, 2022

@adzlocal
I'm sorry for the late reply. I'll check.

@adzlocal
Copy link
Contributor Author

@lja1018 There is a version difference between the base package and the vue wrapper:

Could you take a look? Thanks.

@lja1018
Copy link
Contributor

lja1018 commented Oct 13, 2022

@adzlocal
Let me check. Thank you.

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