Summary: | Add "Trim page to selection" feature | ||
---|---|---|---|
Product: | [Applications] okular | Reporter: | Jake Linder <JakeLinder> |
Component: | general | Assignee: | Okular developers <okular-devel> |
Status: | RESOLVED FIXED | ||
Severity: | wishlist | CC: | aacid, simonandric5, tso2000 |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | Other | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/okular/172d78c6b3f9d7894a1233fb12e5e292f13710ee | Version Fixed In: |
Description
Jake Linder
2015-08-10 14:23:51 UTC
1. just make it a new tool? 2. What would that menu item do? 3. How is it actually applied? I mean it is applied if you save the data for it to be applied. 4. I never liked Trim Margins and it's a can of bugs, expanding in that direction is probably going to introduce even more bugs. Is this really something someone else than you is going to use? 1. Unlike other tools, it is also modal -- applying it changes the view, and deselecting it shouldn't disable it. You may want to trim an article to read it, then annotate as you read. 2. Show that the mode is activated. Allow Deactivation. Possibly trigger the selection tool. 3. I've followed the "Trim Margins" code closely and introduced an entry into the settings. I think the save/restore logic is somehow part of that but I don't see where I need to tweak it. 4. How can I answer that? I do like it. I do use it. All software has bugs. Any new feature introduces bugs. Bugs can usually be fixed (unless they are due to bad design). Yes, I find it extremely useful and yes I think others would too. Other tickets on this bugtracker show that users are making use of "Trim margins", mention its usefulness, and there are multiple requests for increased functionality in that direction, including what I'm suggesting. Here's an example file for which "Trim Margins" is insufficient: http://ajcn.nutrition.org/content/82/1/69.full.pdf 1. right 2. so maybe make the trim margins a submenu with two items that are mutually exclusive and that you can uncheck? 3. If you don't need to save it, why store it in the settings? (and no i'm not going to be able to give you any more concrete help until you post a review request in reviewboard and there's code to see) 3. Fair enough, that can wait anyway. Assuming the code is reasonably unobtrusive, is there a fair chance this'll be merged if I get it ready? if yes, I'll make the necessary changes and put up the code for your comments. Let's see the code, and then we can talk about it. If it has tests, the chances for merging increase :) Tests! The bane of driveby code contributions.... grrr If you're going to dump some code that i'm have to going to maintain for the next 10 years, you may as well help me a bit with it, no? True. Just about there, I'll let it sleep tonight and have a fresh look again tomorrow to spot any brain-slippage, then i'll up it to RB. I'll add the tests once we're agreed the interaction is ok. Before upping for review: - What branch should I rebase against? - Is there a project-mandated `astyle` incantation (or somth) for code formatting? - What branch should I rebase against? master - Is there a project-mandated `astyle` incantation (or somth) for code formatting? No, just follow the style of the file Review Request is up: https://git.reviewboard.kde.org/r/124716/ *** Bug 350073 has been marked as a duplicate of this bug. *** ping I've seen the review, i'm very busy and since this is a new feature that can't be released until december i'm giving it a bit less priority, don't worry i won't forget Git commit 172d78c6b3f9d7894a1233fb12e5e292f13710ee by Albert Astals Cid, on behalf of Jake Linder. Committed on 27/08/2015 at 20:09. Pushed by aacid into branch 'master'. Add "Trim To Selection" feature Changes C1. Added submenu, moved "Trim margins" (TM mode) to it and added "Trim To Selection" (TS mode). C2. Activating "Trim To selection" enters a new mousemode, similar to RectSelect for defining a viewport. C3. Once a viewport has been defined, it serves as a viewport for all pages in the document. C4. Left/Right pages are not treated differently. Manual Testing T1. Switching between modes enforces at most one active. T2. Can deactivate a mode by selecting it again from the menu. T3. When draggin bbox selection, clicking outside page does not crash. T4. When in "Facing Pages" mode, mouse release must be over any page (or is ignored). T5. Normalized bbox coords are computed relative to page indicated by point of mouse release. T6. Behave as expected when switching between any pair of No Trim/Trim Margins/Trim To Selection. T7. TM mode persisted across app restarts (existing behavior). T8. TS mode forgotten across app restarts (as desired). T9. Exiting and reselectin "Trim To Selection" prompts for new bbox. T10. Choosing a small Trim bbox enforces minimium dimensions size (As percentag of total), as it does in TM mode, because of the "scale big and crop down" implementation, to avoid huge pixmaps. TS mode minimum set at 20% (vs. TM mode's 50%). REVIEW: 124716 M +9 -0 conf/okular.kcfg M +2 -2 part-viewermode.rc M +2 -2 part.rc M +162 -16 ui/pageview.cpp M +4 -0 ui/pageview.h http://commits.kde.org/okular/172d78c6b3f9d7894a1233fb12e5e292f13710ee |