Add 'Two Up View' #2395
Add 'Two Up View' #2395
Conversation
Use param twoup=true
|
General feedback:
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/9c390c91a523cdd/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/9c390c91a523cdd/output.txt Total script time: 0.15 mins Published |
| @@ -30,6 +30,8 @@ var MAX_SCALE = 4.0; | |||
| var IMAGE_DIR = './images/'; | |||
| var SETTINGS_MEMORY = 20; | |||
| var ANNOT_MIN_SIZE = 10; | |||
| var TWO_UP_VIEW = false; | |||
|
Adobe Reader has the first page displayed by itself in two-up mode. |
By default will be good. But we still need to provide options to switch that off (as Reader does) |
Added 'One Page View', 'Two Page View' and 'Show Cover Page in Two Page View'. Added to context menu as sub menu under 'Page Display' (same as Adobe Reader X). Have used icon image to indicate which option selected (by default 'One Page View' and 'Show Cover Page in Two Page View' are selected) Can also use param #showcover=true|false The context menu icon is a hardcoded path in both html and js, should/ could this be done in css? Also an issue if you zoom in so that the width of 2 pages is greater than viewport. Any suggestions? Expanding the sidebar also can result in the same problem
Due to formatting with vs2012
| </menu> | ||
|
|
||
|
|
There was a problem hiding this comment.
Nitpicking: There's an extra line in here that consists only of whitespace.
|
Too many unrelated to the issue change in the code -- the pull request more likely to be rejected as is. Keep the solution short and change only code that needs to be changed; revert the changes made to unrelated code. |
|
|
||
| <menu label="Page Display"> | ||
| <menuitem label="One Page View" id="one_page_view" | ||
| data-l10n-id="one_page_view" icon="images/contextMenu-tick.png" ></menuitem> |
There was a problem hiding this comment.
does the 'checked' attribute not working? https://developer.mozilla.org/en-US/docs/XUL/menuitem#a-checked
There was a problem hiding this comment.
I've attempted to use the checked attribute, but having difficulty toggling the 'One page..' and 'Two page..' options. I have looked at https://developer.mozilla.org/en-US/docs/XUL/PopupGuide/MenuItems and https://developer.mozilla.org/en-US/docs/XUL/menuitem#a-checked but oncommand and/or autocheck="false" doesn't seem allow me to grammatically check or uncheck the states
Fixed zoom where width of 2 pages is greater than viewport Attempted to use checkbox for menuitem, though cannot programatically toggle 'One page' and 'Two page' view, possibly a bug? I've tried using oncommand - will not fire, also autocheck="false" doesn't seem to do anything
Changed menuitem type to "radio"
Now inserts br tags inbetween pages to ensure correct positioning
|
Can someone explain why this has not been merged yet? It seems like a really useful feature to me and I would personally love to use it... |
|
Some comments were never addressed, it seems. Most notably @yurydelendik's comment that the PR includes changes unrelated to the feature being implemented. |
|
@Moistly are you still working on this? Or should someone else (like me) finish what you started? |
|
I would be than happy for someone else to finish this off. Thanks |
|
Close this as I superseded it with #2660? |
|
It can be reopened later if it's necessary. |
Use param twoup=true