Skip to content

Zoom in and out image #1448 #2809

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.

Sign up for GitHub

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

Merged
Rokt33r merged 7 commits into from
Jan 28, 2019
Merged

Zoom in and out image #1448 #2809

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f566b56
add zoom-in and zoom-out action of images #1448
roottool Jan 15, 2019
72df418
DRY my code #1448
roottool Jan 16, 2019
ea5970a
change a magnification by image size #1448
roottool Jan 16, 2019
294bf74
change variable name #1448
roottool Jan 16, 2019
33fb030
Reduced the count of calculation
roottool Jan 25, 2019
1683d63
Modified indent
roottool Jan 25, 2019
5e87ec2
Specified MarkdownPreview class
roottool Jan 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions browser/components/MarkdownPreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,82 @@ export default class MarkdownPreview extends React.Component {
)
}
)

document.querySelectorAll('iframe').forEach(item => {
Copy link
Member

@ZeroX-DG ZeroX-DG Jan 22, 2019

Choose a reason for hiding this comment

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

Why do you need to select all iframes like this? The markdown preview only contains 1 iframe, then why did you have to find all iframes and loop through them? Can you explain this to me?

Copy link
Contributor Author

@roottool roottool Jan 24, 2019

Choose a reason for hiding this comment

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

Explanation

The code is my mistake.
Because when I tried to catch image elements into iframe, I used the code as it is without thinking.

Suggestion

I have a suggestion to resolve it.
suggestion
document.querySelector() searches for an element from entire HTML.
I considered the possibility of adding new iframes in Boostnote.
So I use a classname of a markdown preview iframe.

It has a pros and cons.
Pros

  • Specifying an element is easy because MarkdownPreview used only here.

Cons

  • If a classname changes from MarkdownPreview, a classname using document.querySelector() will need to change too.

Before

    document.querySelectorAll('iframe').forEach(item => {
      const rect = item.getBoundingClientRect()
      const imgList = item.contentWindow.document.body.querySelectorAll('img')

After

    const markdownPreviewIframe = document.querySelector('.MarkdownPreview')
    const rect = markdownPreviewIframe.getBoundingClientRect()
    const imgList = markdownPreviewIframe.contentWindow.document.body.querySelectorAll('img')

Copy link
Member

@ZeroX-DG ZeroX-DG Jan 24, 2019

Choose a reason for hiding this comment

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

We can just go for the 2nd method, I don't think there's any reason to change the MarkdownPreview class name. In case we were going to change it, a simple search & replace for the MarkdownPreview class wouldn't be hard.

Copy link
Contributor Author

@roottool roottool Jan 24, 2019

Choose a reason for hiding this comment

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

I was worried about changing only in HTML.
I want to modify from before code to after code because I feel relieved by your reply.

Copy link
Member

@ZeroX-DG ZeroX-DG Jan 27, 2019

Choose a reason for hiding this comment

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

Yes, you can just go with the after code 👍

Copy link
Contributor Author

@roottool roottool Jan 27, 2019

Choose a reason for hiding this comment

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

OK, I rewrote to after code.
Please check the code again.

const rect = item.getBoundingClientRect()
const imgList = item.contentWindow.document.body.querySelectorAll('img')
for (const img of imgList) {
img.onclick = () => {
const widthMagnification = document.body.clientWidth / img.width
const heightMagnification = document.body.clientHeight / img.height
const baseOnWidth = widthMagnification < heightMagnification
const magnification = baseOnWidth ? widthMagnification : heightMagnification

const zoomImgWidth = img.width * magnification
const zoomImgHeight = img.height * magnification
const zoomImgTop = document.body.clientHeight / 2 - zoomImgHeight / 2
Copy link
Member

@ZeroX-DG ZeroX-DG Jan 22, 2019

Choose a reason for hiding this comment

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

Little suggestion, you can transform this code into:

const zoomImgTop = (document.body.clientHeight - zoomImgHeight) / 2

This way, we only need to do 2 calculation instead of 3

Copy link
Contributor Author

@roottool roottool Jan 24, 2019

Choose a reason for hiding this comment

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

I didn't notice it.
I am going to fix it tomorrow.
Thank you for teaching it!

Copy link
Member

@ZeroX-DG ZeroX-DG Jan 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

No problem 👍 This is a great feature!

const zoomImgLeft = document.body.clientWidth / 2 - zoomImgWidth / 2
const originalImgTop = img.y + rect.top
const originalImgLeft = img.x + rect.left
const originalImgRect = {
top: `${originalImgTop}px`,
left: `${originalImgLeft}px`,
width: `${img.width}px`,
height: `${img.height}px`
}
const zoomInImgRect = {
top: `${baseOnWidth ? zoomImgTop : 0}px`,
left: `${baseOnWidth ? 0 : zoomImgLeft}px`,
width: `${zoomImgWidth}px`,
height: `${zoomImgHeight}px`
}
const animationSpeed = 300

const zoomImg = document.createElement('img')
zoomImg.src = img.src
zoomImg.style = `
position: absolute;
top: ${baseOnWidth ? zoomImgTop : 0}px;
left: ${baseOnWidth ? 0 : zoomImgLeft}px;
width: ${zoomImgWidth};
height: ${zoomImgHeight}px;
`
zoomImg.animate([
originalImgRect,
zoomInImgRect
], animationSpeed)

const overlay = document.createElement('div')
overlay.style = `
background-color: rgba(0,0,0,0.5);
cursor: zoom-out;
position: absolute;
top: 0;
left: 0;
width: 100%;
height: ${document.body.clientHeight}px;
z-index: 100;
`
overlay.onclick = () => {
zoomImg.style = `
position: absolute;
top: ${originalImgTop}px;
left: ${originalImgLeft}px;
width: ${img.width}px;
height: ${img.height}px;
`
const zoomOutImgAnimation = zoomImg.animate([
zoomInImgRect,
originalImgRect
], animationSpeed)
zoomOutImgAnimation.onfinish = () => overlay.remove()
}

overlay.appendChild(zoomImg)
document.body.appendChild(overlay)
}
}
})
}

focus () {
Expand Down
1 change: 1 addition & 0 deletions browser/components/markdown.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ p
white-space pre-line
word-wrap break-word
img
cursor zoom-in
max-width 100%
strong, b
font-weight bold
Expand Down