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(web-react): Recalculate FileUploader image preview by crop values #1124

Merged
merged 4 commits into from
Nov 2, 2023

Conversation

literat
Copy link
Collaborator

@literat literat commented Nov 1, 2023

Description

  • need to use additional dimensions - natural width and height to
    calculate dimensions of previewed image
  • also need to better calculate the scale of the entire image and also
    position when we need to display the possible crop

Additional context

https://almamedia.slack.com/archives/C050YPZ3YRW/p1698830929192449

Issue reference

https://jira.lmc.cz/browse/DS-1038


Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Follow the PR Title/Commit Message Convention.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

Copy link

netlify bot commented Nov 1, 2023

Deploy Preview for spirit-design-system-storybook ready!

Name Link
🔨 Latest commit 27a3a13
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-storybook/deploys/65439396fadc080008eaf943
😎 Deploy Preview https://deploy-preview-1124--spirit-design-system-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 1, 2023

Deploy Preview for spirit-design-system-react ready!

Name Link
🔨 Latest commit 27a3a13
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-react/deploys/65439396ebc0cf00084a2664
😎 Deploy Preview https://deploy-preview-1124--spirit-design-system-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the bug Something isn't working label Nov 1, 2023
Copy link

nx-cloud bot commented Nov 1, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 27a3a13. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 3 targets

Sent with 💌 from NxCloud.

Copy link

netlify bot commented Nov 1, 2023

Deploy Preview for spirit-design-system-demo ready!

Name Link
🔨 Latest commit 27a3a13
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-demo/deploys/65439396ebc0cf00084a265f
😎 Deploy Preview https://deploy-preview-1124--spirit-design-system-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 1, 2023

Deploy Preview for spirit-design-system-validations canceled.

Name Link
🔨 Latest commit 27a3a13
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-validations/deploys/654393967de4a60008df3b40

Copy link
Contributor

github-actions bot commented Nov 1, 2023

Coverage Status

coverage: 69.129% (-1.4%) from 70.548% when pulling ac712c1 on fix/DS-1038-fileuploader-meta-crop into e07eb5a on main.

Copy link
Contributor

github-actions bot commented Nov 1, 2023

Coverage Status

coverage: 69.151% (-1.4%) from 70.548% when pulling 6545d9c on fix/DS-1038-fileuploader-meta-crop into e07eb5a on main.

Copy link
Contributor

@adamkudrna adamkudrna left a comment

Choose a reason for hiding this comment

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

This is my first impression, haven't tested it yet.

Copy link
Contributor

github-actions bot commented Nov 2, 2023

Coverage Status

coverage: 68.796% (-1.8%) from 70.548% when pulling fa702ca on fix/DS-1038-fileuploader-meta-crop into e07eb5a on main.

@literat literat force-pushed the fix/DS-1038-fileuploader-meta-crop branch from fa702ca to fb17044 Compare November 2, 2023 10:55
Copy link
Contributor

@adamkudrna adamkudrna left a comment

Choose a reason for hiding this comment

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

Still haven't tested (as we said, it's not easy), but it looks good. 👍🏻

  * need to use additional dimensions - natural width and height to
    calculate dimensions of previewed image
  * also need to better calculate the scale of the entire image and also
    position when we need to display the possible crop

refs #DS-1038
  * need to use additional dimensions - natural width and height to
    calculate dimensions of previewed image
  * also need to better calculate the scale of the entire image and also
    position when we need to display the possible crop

refs #DS-1038
  * allow larger base64 image to increase quality of previewed image

refs #DS-1039
  * allow larger base64 image to increase quality of previewed image

refs #DS-1039
@literat literat force-pushed the fix/DS-1038-fileuploader-meta-crop branch from fb17044 to 27a3a13 Compare November 2, 2023 12:18
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Coverage Status

coverage: 68.796% (-1.8%) from 70.548% when pulling 27a3a13 on fix/DS-1038-fileuploader-meta-crop into e07eb5a on main.

@literat literat merged commit ccbd6c0 into main Nov 2, 2023
19 checks passed
@literat literat deleted the fix/DS-1038-fileuploader-meta-crop branch November 2, 2023 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants