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 an issue where exported images would be smaller #1349

Closed

Conversation

hatsu38
Copy link
Contributor

@hatsu38 hatsu38 commented Mar 13, 2022

  • Integration tests (if applicable)

Closes #1287

Issue Summary

When viewing on mobile, increasing width makes it responsive.
However, the exported image was also responsive and vertically long.
Detail: #1287 (comment)

Solution

  • When the set width is larger than the displayed width, the width of the exported image is calculated using the set width.
  • Changed the display from responsive to scrolling
2022-03-13.15.58.52-1.mov

Calculates the Width of the image to be exported when the widthAdjustment is False and the set Width
is greater than the displayed Width.
@vercel
Copy link

vercel bot commented Mar 13, 2022

@hatsu38 is attempting to deploy a commit to the Carbon Team on Vercel.

A member of the Team first needs to authorize it.

Comment on lines +99 to +103
const settingWidth = parseInt(this.state.width)
const isOverWidth = offsetWidth < settingWidth
if (!isOverWidth) return offsetWidth

return settingWidth + parseInt(this.state.paddingHorizontal)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the set width is larger than the displayed width, the width of the exported image is calculated using the set width.

@hatsu38 hatsu38 marked this pull request as ready for review March 13, 2022 07:21
Copy link
Contributor

@mfix22 mfix22 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay @hatsu38

@@ -93,6 +93,16 @@ class Editor extends React.Component {
updateCode = code => this.updateState({ code })
updateWidth = width => this.setState({ widthAdjustment: false, width })

calculateExportImageBaseWidth = offsetWidth => {
Copy link
Contributor

Choose a reason for hiding this comment

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

@hatsu38 sorry for the delay. Unfortunately, I don't want to do any complex calculation here.

I would prefer for the editor window to go off screen on smaller displays and have this function just capture what is there than try and make the editor fit on smaller screens and do this calculation after the fact.

Can we just make the editor go off screen on mobile, but allow the user to easily scroll horizontally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mfix22 Thank you for your comment.
I will fix it at a later date and push it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hatsu38 no rush on this at all, but just wanted to check in here to see if you have any updates 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I have not been able to solve this problem recently due to lack of time

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries at all @hatsu38! I'm going to close this in the meantime, but feel free to re-open it whenever, or open a new PR anytime.

@mfix22 mfix22 closed this May 16, 2022
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.

Web app is unusable on mobile
2 participants