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

Image resize in % doesn't work anymore #1115

Closed
1 task done
costimuraru opened this issue Apr 13, 2020 · 7 comments · Fixed by #1116
Closed
1 task done

Image resize in % doesn't work anymore #1115

costimuraru opened this issue Apr 13, 2020 · 7 comments · Fixed by #1116

Comments

@costimuraru
Copy link

costimuraru commented Apr 13, 2020

Bug Report

We're following the docsify documentation, which states that it's possible to resize an image using a percentage: https://github.com/docsifyjs/docsify/blob/develop/docs/helpers.md#resizing

Steps to reproduce

Use markdown to insert an image, with 50% width:

![My diagram](/_media/diagrams/sample_upstream_flow.png ':size=50%')

What is current behaviour

Notice the width is wrong. Instead of 50% it's just 50. Also the % seems to somehow be transferred to the title attribute:

<img src="/upstream-protocol/sample-app/_media/diagrams/sample_upstream_flow.png" 
  alt="My diagram" 
  title="%" 
  width="50" 
  height="50">

What is the expected behaviour

<img src="/upstream-protocol/sample-app/_media/diagrams/sample_upstream_flow.png" 
  alt="My diagram"
  width="50%">

Other relevant information

Screenshot 2020-04-13 16 18 11

Could this be a parsing mardown issue? Perhaps here: https://github.com/docsifyjs/docsify/blob/master/src/core/render/utils.js#L6-L14 ?

  • Bug does still occur when all/other plugins are disabled?

  • Your OS:

  • Node.js version:

  • npm/yarn version:

  • Browser version:

  • Docsify version: 4.11.3

  • Docsify plugins:

Please create a reproducible sandbox

Edit 307qqv236

Mention the docsify version in which this bug was not present (if any)

@costimuraru
Copy link
Author

CC @anikethsaha, @QingWei-Li

@anikethsaha
Copy link
Member

I think this #1065 might fix the height issue.
And about the % issue, I do think there might be some bug with the extractor.

could you try ![My diagram](/_media/diagrams/sample_upstream_flow.png ':size=50%x50%') and check if the % issue still exists or not !

@Koooooo-7
Copy link
Member

I think this #1065 might fix the height issue.
And about the % issue, I do think there might be some bug with the extractor.

yep, I think it is the regex issues.

...
.replace(/(?:^|\s):([\w-]+:?)=?([\w-]+)?/g, (m, key, value) => {

the ([\w-]+)missed the %. and I checked v4.11.0,v4.8.0,v4.6.8, both have this problem 😅.
I fixed the regex

 .replace(/(?:^|\s):([\w-]+:?)=?([\w-%]+)?/

then, it works.
image

@anikethsaha
Copy link
Member

@Koooooo-7 thanks a lot for investigating this ( as always 😄 )

Can you submit a PR with the fix and also we need to check if it's breaking something else or not,

@Koooooo-7
Copy link
Member

@Koooooo-7 thanks a lot for investigating this ( as always )

Can you submit a PR with the fix and also we need to check if it's breaking something else or not,

yea, I m working on it. 🔨

anikethsaha added a commit that referenced this issue Apr 13, 2020
[fix #1115] Image resize in % doesn't work.
@costimuraru
Copy link
Author

Thanks for the quick fix here, @Koooooo-7. And @anikethsaha for reviewing. You guys rock!

@cugxuan
Copy link

cugxuan commented May 25, 2020

so great!
waiting for new release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants