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

Enhance zoom #578

Merged
merged 7 commits into from
Oct 9, 2016
Merged

Enhance zoom #578

merged 7 commits into from
Oct 9, 2016

Conversation

im-mi
Copy link
Contributor

@im-mi im-mi commented Sep 2, 2016

New features

  • Added zoom support to webm, svg, and ico
    • Note that this makes webm no longer stretch to 100% width. An additional zoom mode could be added if the old behavior is desired.
  • Re-apply zoom upon window resize
    • This affects the "Fit Height" and "Fit Both" zoom modes. Before, the user would have to refresh the page or toggle zoom settings to get the zoom to update.
  • When loading an SVG, it now uses the size of the outermost SVG element. Before, the size could be reported incorrectly when there were nested SVG elements.

Considerations

  • The code that handles zooming is part of the handle_pixel extension. Making it handle other formats may be considered a breach of responsibility. Perhaps the zoom functionality could be moved to its own extension.
  • This expands the usage of .shm-main-image and #main_image to other types of main post content. This has implications for CSS because now post content can be styled at once, regardless of whether it is pixel, webm, svg, or ico. Personally, I feel that this is a good change.

Limitations

  • I didn't expand support to flash-based video. I can't test it (flash video doesn't work for me), and its height is hard-coded to 480px which makes me feel that changing it may break something.
  • Click-to-zoom is disabled for webm so that the player controls are still accessible.
  • Click-to-zoom is disabled for SVG because said format can contain interactive content and I couldn't get SVG click detection to work in any browser anyways.

Issues

  • Supporting zooming means that the content's width and height can't be set directly in the HTML. This may cause jitteriness because then determining the content's size is deferred until handle_pixel's zoom script runs. Note that this issue already affected pixel images.
  • The SVG theme uses a double-elemented method of embedding SVGs (<object><embed/><object>). This could potentially cause issues in some browsers because it's currently set such that the zoom code only interacts with the outer <object> element. Regardless, it works on the latest desktop Chrome, Edge, and Firefox.

@im-mi
Copy link
Contributor Author

im-mi commented Sep 3, 2016

Changes:

  • Added zoom support to SVG
  • When loading an SVG, it now uses the size of the outermost SVG element. Before, the size could be reported incorrectly when there were nested SVG elements.

Notes:

  • Click-to-zoom is disabled for SVG because said format can contain interactive content and I couldn't get SVG click detection to work in any browser anyways.
  • The SVG theme uses a double-elemented method of embedding SVGs (<object><embed/><object>). This could potentially cause issues in some browsers because it's currently set such that the zoom code only interacts with the outer <object> element. Regardless, it works on the latest desktop Chrome, Edge, and Firefox.

@im-mi
Copy link
Contributor Author

im-mi commented Sep 22, 2016

Nothing left to add here.

@shish
Copy link
Owner

shish commented Oct 9, 2016

All looks good /o/ Sorry for taking forever to review this >_<

@shish shish merged commit 7548c66 into shish:develop Oct 9, 2016
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.

2 participants