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

Prevents zooming in the rendered html #30

Closed
wants to merge 1 commit into from
Closed

Prevents zooming in the rendered html #30

wants to merge 1 commit into from

Conversation

Kumuluzz
Copy link
Contributor

@Kumuluzz Kumuluzz commented May 17, 2017

Addresses issue #16

I came across the need to disable zoom in my DownView. I thought of two solutions:

  1. Disable zoom per default
  2. Inject a flag when initializing the DownView which enables/disables zoom

This pull request implements option 1) since I imagine this being the most common

@Kumuluzz Kumuluzz changed the title #16: Prevents zooming in the rendered html Prevents zooming in the rendered html May 17, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.623% when pulling f06b270 on Kumuluzz:master into 43241e5 on iwasrobbed:master.

@iwasrobbed
Copy link
Collaborator

Hi @Kumuluzz 👋

I appreciate the pull request, but I'd rather not change the default zooming for everyone. I tend to prefer configuration with sensible defaults (more like your option 2).

Now that Down supports instantiation with a custom bundle, it might be best to just do it that way for now. If a lot of people end up wanting this, I'm happy to have another look later on.

@iwasrobbed iwasrobbed closed this May 17, 2017
@Kumuluzz
Copy link
Contributor Author

Hi @iwasrobbed!

Do you think it would make sense if I update my pull request to have a predefined bundle that can be specified when initializing the DownView? Fx by expanding the init method with a allowZooming parameter:

public init(frame: CGRect, markdownString: String, openLinksInBrowser: Bool = true, didLoadSuccessfully: DownViewClosure? = nil, allowZooming: Bool = true)

Otherwise I can simply update my pull request to update the documentation on how to disable zooming 😊

@iwasrobbed
Copy link
Collaborator

Hey @Kumuluzz! I'd prefer just keeping this as a documentation update for now versus including an additional bundle for this one scenario. If it ends up being a widely needed feature, more people are welcome to comment on this to let us know. Thanks!

@Kumuluzz
Copy link
Contributor Author

@iwasrobbed I'll update the documentation with some instructions then. Any preferences on where to place it? Just directly in the README below ### Options?

@iwasrobbed
Copy link
Collaborator

Maybe underneath the View Rendering section since it's specific to that area?

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.

None yet

3 participants