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

Scale images based on Pandoc classes #3608

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

zzccppp
Copy link

@zzccppp zzccppp commented Jul 14, 2022

Description

This PR is for scale images in issue #1328. Documents imported with pandoc usually contain image size information, such as:
![](image1.png){width="3.69in" height="1.93884in"}
So just parse the information in braces and pass it to the corresponding attribute to control image size.

Changes

  • parse the parameters in the braces and pass them to image.style.maxWidth attribute

Additional information

Tested on: macOS 12.4 Monterey Apple Silicon

@boring-cyborg
Copy link

boring-cyborg bot commented Jul 14, 2022

Thank you for opening your first PR! 🎉 We are very happy and would like to thank you very much for your contribution. If everything checks out, we'll make sure to review the PR as soon as possible and give feedback. In the meantime, to make the reviewing process as fast as possible, you can help us by checking the following things:

  • Did you follow the JSStandard coding style? - Did you comment everywhere where the necessity of a piece of code or the
    way it was implemented is not immediately obvious?
  • Did you attempt to stick as much to current coding habits as possible?
    (Note that this does not apply to pieces of code where we ourselves
    obviously violated good coding practices, which unfortunately happens
    sometimes. But please indicate this in your PR so that we know what you
    rectified!)

Furthermore, make sure that the linter does not complain, which will check your code on every new commit. If the linter task fails, make sure to run yarn lint locally and check the file eslint_report.htm which will tell you precisely what went wrong.
Stay sharp, and thanks again!

Copy link
Member

@nathanlesage nathanlesage left a comment

Choose a reason for hiding this comment

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

In general looks good, however a few questions:

  1. Unfortunately, GitHub displayed the file as if it had been removed and completely written from scratch. After turning off Whitespace change, it worked like a charm. Did you accidentally exchange, e.g., spaces with tabs, or LF with CRLF…?
  2. There are two critical issues we have to think about: (a) the metrics question I've already posed, and (b) how to handle the settings' maxPreviewWidth and maxPreviewHeight?

let heightAttribute = ''
if (match[4] !== '') {
// Use regex to parse pandoc attributes
// like width="3.69in" height="1.93884in"
Copy link
Member

Choose a reason for hiding this comment

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

Two questions: 1) are quotes required? If not one should account for that in below's regexp. 2) Regarding size attributes: Obviously, for preview-reasons we don't need to accurately represent inches etc., but what are the options for measures people have, and how should we maybe account for that? (50% width is easy to accomplish, but how much are 3.5 inches relative to the editor area?)

Copy link
Author

Choose a reason for hiding this comment

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

I try to use pandoc to import docx documents. The attribute generated by pandoc is in inches. Every available length unit in CSS images can be used here. For the absolute length units (cm mm in px), its size is fixed, does not change with the window, and corresponds to the same px (1in = 96px).
As for the maxPreviewWidth and maxPreviewHeight, if absolute length units are used, conversion is required. I don't know how to calculate it.

@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale This label indicates that this issue might be automatically closed soon. label Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale This label indicates that this issue might be automatically closed soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants