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

default html img resize if no height included #1065

Merged
merged 2 commits into from
May 18, 2020
Merged

default html img resize if no height included #1065

merged 2 commits into from
May 18, 2020

Conversation

maubuz
Copy link
Contributor

@maubuz maubuz commented Mar 11, 2020

Summary

When image is resized by providing only a width, it does not set the height. Let html resize automatically to respect the aspect ratio.

Fixes #1064 .

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:


  • DO NOT include files inside lib directory.

@maubuz
Copy link
Contributor Author

maubuz commented Mar 11, 2020

Unit test suite (build 12.x) is failing in render.test.js, line 171 because test expects the output to include a height when only a width is provided.

Should the test be adjusted?

@anikethsaha
Copy link
Member

Unit test suite (build 12.x) is failing in render.test.js, line 171 because test expects the output to include a height when only a width is provided.

Should the test be adjusted?

yes

Copy link
Member

@anikethsaha anikethsaha left a comment

Choose a reason for hiding this comment

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

💯

@anikethsaha
Copy link
Member

anikethsaha commented Mar 11, 2020

I am not sure whether I can release this soon. Maybe this weekend!
so I would advice is you want this to be changed, you can try

![](<link here> ':width=100')

@maubuz
Copy link
Contributor Author

maubuz commented Mar 11, 2020

Thank you! I don't mind waiting. For now, I'll just point my index.html to my compiled version of docsify.min.js.
Happy to contribute to such a great project!

Also, I can't seem to get the syntax ':width=100' to work. Is it currently supported?

@anikethsaha
Copy link
Member

Also, I can't seem to get the syntax ':width=100' to work. Is it currently supported?

oops...I we dont have support for that. I thought we had support for building dynamic attributes from these markdown syntax.

I guess we can have that, that would solve many similar issues

@arelstone
Copy link
Contributor

I think this looks legit to me. Didn't think of this when i did the refactoring

@jihao
Copy link

jihao commented Apr 11, 2020

Please, can someone merge this pull request?
I had to change to older version which works fine.

<script src="//unpkg.com/docsify@4.10.2/lib/docsify.js"></script>

@maubuz
Copy link
Contributor Author

maubuz commented Apr 11, 2020

Let me know if there is anything required from my end to complete the merge.
A test is failing with a timeout error but I don't see the connection with this PR.

@maubuz
Copy link
Contributor Author

maubuz commented May 6, 2020

I did a rebase on top of latest develop branch. Nothing was changed in the code.
All tests are passing now.

@anikethsaha anikethsaha added the semver-patch This needs a patch release label May 18, 2020
@anikethsaha anikethsaha merged commit 9ff4d06 into docsifyjs:develop May 18, 2020
@anikethsaha
Copy link
Member

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch This needs a patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image resizing with Markdown assumes height = width and distorts image
4 participants