D21490: Draw line caption for non-PDF documents

[Tobias Deiminger]

tobiasdeiminger added a comment.


  Does anybody feel like adding a nice UI for this feature? :-)
  
  In it's simplest form it would be a "show caption" checkbox in the straight 
line tool.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21490

To: tobiasdeiminger
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21416: Add icons for line annotation arrow styles to combo box

[Tobias Deiminger]

tobiasdeiminger marked an inline comment as done.
tobiasdeiminger added a comment.


  In D21416#471795 , @davidhurka 
wrote:
  
  > For dark color themes: maybe the color of the adjacent text would be good 
as foreground color for the icons. Don’t know, but maybe 
QGuiApplication::pallete().color(QPalette::WindowText)?
  
  
  Makes sense, thanks. Never tried this before, give me some time to check it 
out...

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21416

To: tobiasdeiminger, #okular
Cc: davidhurka, knambiar, ngraham, okular-devel, joaonetto, tfella, darcyshen, 
aacid

D21490: Draw line caption for non-PDF documents

[Tobias Deiminger]

tobiasdeiminger edited the summary of this revision.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21490

To: tobiasdeiminger
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21248: Add line annotation ending arrows for non PDF documents

[Tobias Deiminger]

tobiasdeiminger added a dependent revision: D21490: Draw line caption for 
non-PDF documents.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21248

To: tobiasdeiminger, #okular, aacid
Cc: ngraham, sander, knambiar, okular-devel, joaonetto, tfella, darcyshen, aacid

D21490: Draw line caption for non-PDF documents

[Tobias Deiminger]

tobiasdeiminger created this revision.
Herald added a project: Okular.
Herald added a subscriber: okular-devel.
tobiasdeiminger requested review of this revision.

REVISION SUMMARY
  Line captions are already implemented in poppler / PDF generator (can't be 
enabled in okular UI yet, but that's another story).
  
  This adds the drawing code for non-PDF documents.
  
  TODO:
  
  - obey inline / top setting
  - precombine transformation matrix
  - if inline, partially spare out main segment
  - figure out which font size and color to use
  - adjust placement under some conditions (e.g. avoid upside down text)?
  
  Depends on D21248 

REPOSITORY
  R223 Okular

BRANCH
  feature/linecaption

REVISION DETAIL
  https://phabricator.kde.org/D21490

AFFECTED FILES
  ui/pagepainter.cpp
  ui/pagepainter.h

To: tobiasdeiminger
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21416: Add icons for line annotation arrow styles to combo box

[Tobias Deiminger]

tobiasdeiminger updated this revision to Diff 58872.
tobiasdeiminger added a comment.


  Define TermStyle list outside the loop.

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21416?vs=58688=58872

BRANCH
  feature/lineannoticons

REVISION DETAIL
  https://phabricator.kde.org/D21416

AFFECTED FILES
  ui/annotationwidgets.cpp
  ui/annotationwidgets.h

To: tobiasdeiminger, #okular
Cc: davidhurka, knambiar, ngraham, okular-devel, joaonetto, tfella, darcyshen, 
aacid

D10859: Improve layout of annotation configuraton dialogs

[David Hurka]

davidhurka added a comment.


  In D10859#471799 , @ngraham wrote:
  
  > I don't think the section headings really added much. Most of them were 
group boxes with one item, which is pointless. When using a FormLayout style,  
in general you want to make the labels self-explanatory as much as possible so 
you don't need section headers. To separate groups of related settings, you can 
add some whitespace that has the effect of grouping things together in a very 
natural-looking way. We do this extensively in other FormLayouts and people 
generally think it looks good:
  
  
  I’m not sure whether to agree.
  
  Your screenshots show vertical whitespace, but between differently shaped 
widgets. That makes the space appear a bit random. Then I looked in System 
Settings, which uses mostly group boxes, and sometimes space, section labels, 
and horizontal lines.
  
  But in this dialog, whitespace will probably look good, because all widgets 
have the same shape.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D10859

To: simgunz, #okular, #vdg, ngraham
Cc: davidhurka, aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, 
darcyshen

D10859: Improve layout of annotation configuraton dialogs

[Nathaniel Graham]

ngraham added a comment.


  I don't think the section headings really added much. Most of them were group 
boxes with one item, which is pointless. When using a FormLayout style,  in 
general you want to make the labels self-explanatory as much as possible so you 
don't need section headers. To separate groups of related settings, you can add 
some whitespace that has the effect of grouping things together in a very 
natural-looking way. We do this extensively in other FormLayouts and people 
generally think it looks good:
  F6858748: Screenshot_20190529_135149.png 

  F6858751: Screenshot_20190529_135305.png 


REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D10859

To: simgunz, #okular, #vdg, ngraham
Cc: davidhurka, aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, 
darcyshen

D21416: Add icons for line annotation arrow styles to combo box

[David Hurka]

davidhurka added a comment.


  The UI looks good and I understand the code. :)
  
  For dark color themes: maybe the color of the adjacent text would be good as 
foreground color for the icons. Don’t know, but maybe 
QGuiApplication::pallete().color(QPalette::WindowText)?

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21416

To: tobiasdeiminger, #okular
Cc: davidhurka, knambiar, ngraham, okular-devel, joaonetto, tfella, darcyshen, 
aacid

D10859: Improve layout of annotation configuraton dialogs

[David Hurka]

davidhurka added a comment.


  Just tried this. Maybe you can bring back the section headings, which were 
group box labels before? Straight Line got many options recently.
  
  Not working buddys (probably a problem with PixmapPreviewSelector):
  Popup-Note -> Icon (Alt + i)
  Stamp -> Stamp symbol (Alt + s)
  (Another not working accelerator: Configure Okular -> Annotations: Author and 
Add both have Alt + a.)
  
  In D10859#471480 , @simgunz wrote:
  
  > Regarding the stamp annotation tool, I have aligned the combobox to the 
top, because I have not found a way to vertically align the label.
  
  
  Looks ok to me.

INLINE COMMENTS

> annotationwidgets.cpp:378
> +m_pixmapSelector = new PixmapPreviewSelector( widget );
> +formlayout->addRow( i18n( "Stamp symbol:" ), m_pixmapSelector );
>  m_pixmapSelector->setEditable( true );

Capitalize symbol?

> annotationwidgets.cpp:379
> +formlayout->addRow( i18n( "Stamp symbol:" ), m_pixmapSelector );
>  m_pixmapSelector->setEditable( true );
>  

Why is this editable? Entering a path to an image does not work.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D10859

To: simgunz, #okular, #vdg, ngraham
Cc: davidhurka, aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, 
darcyshen

D10859: Improve layout of annotation configuraton dialogs

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> ngraham wrote in annotationwidgets.cpp:206
> For this one, `FieldsStayAtSizeHint` might be better, and then we just set 
> sensible sizehints for anything that winds up too small. That's basically 
> what we're doing for all our QML software. In general we're trying to move 
> towards making the controls optimally sized in the first place and then 
> static in size when the window grows or shrinks. We often manually set the 
> width of spinboxes, comboboxes, and text fields to make sure they're all the 
> same width.

Let's not have static sizes, this is not a phone screen where you can't resize 
windows.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D10859

To: simgunz, #okular, #vdg, ngraham
Cc: aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, darcyshen

D10859: Improve layout of annotation configuraton dialogs

[Nathaniel Graham]

ngraham added a comment.


  Thanks! Much simpler indeed.

INLINE COMMENTS

> simgunz wrote in annotationwidgets.cpp:206
> Is it correct to set this growth policy? I have not found any guideline on 
> the HIG. Compared to the grid layout where label and widget take the same 
> amount of horizontal space, now the widgets take way more space making it a 
> little ugly for some annotation tools. Any idea?
> 
> F6857727: Screenshot_20190529_095013.png 
> 

For this one, `FieldsStayAtSizeHint` might be better, and then we just set 
sensible sizehints for anything that winds up too small. That's basically what 
we're doing for all our QML software. In general we're trying to move towards 
making the controls optimally sized in the first place and then static in size 
when the window grows or shrinks. We often manually set the width of spinboxes, 
comboboxes, and text fields to make sure they're all the same width.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D10859

To: simgunz, #okular, #vdg, ngraham
Cc: aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, darcyshen

D21281: [RFC] Write a bit Documentation for Part and PageView

[David Hurka]

davidhurka added a comment.


  Got an overview of open-a-document methods in the Detailed Description of 
Part. Helped me a lot, can it stay there?
  
  I made some method descriptions a bit more robust to future changes (I think) 
by making them more abstact and focusing a bit more on the intention.
  
  Also got a thought that the class documentation should deal with the 
intentions, and less with the implementation. Thanks for your critics, Albert.

INLINE COMMENTS

> part.h:191
> + * @param url Path to the document file.
> + * @param page The number of the page, where 1 is the first page.
> + *

What if the fragment of the URL contains a page number?

> part.h:739
> + */
>  void errorMessage( const QString , int duration = 0 );
> +

A duration of 0 milliseconds seems a bit short to me. Why is this default?

> part.h:773
> + */
>  bool aboutToShowContextMenu(QMenu *menu, QAction *action, QMenu 
> *contextMenu);
> +

This is a function, but sounds like a signal. How about “addBookmarkActions”?

> part.h:780
> + * If @p page is not nullptr, there will be a section with title 
> "Page N",
> + * where N is the page number of @p page. It will contain bookmark 
> and view actions specific to that page.
> + *

One view action is “Fit Width”. Unlike Fit Width from the menu bar, this Fit 
Width will not only set the zoom to Fit Width, but also set the view mode to 
Single Page, and center the viewport on page N.

> davidhurka wrote in part.h:644
> This opens a context menu for context menu items, or what?
> 
> It accepts events of type ContextMenu, that’s clear. To open the context 
> menu, it seems to need a QMenu as event source.

Seems like it opens a context menu for bookmark entries in the Bookmarks menu, 
to rename or delete a single bookmark. But such a menu does not open in my 
Okular.

> davidhurka wrote in part.h:647
> Uh-oh, this is complicated. Would someone look over it?
> 
> Yes, the language is not final.

Should be OK now, also improved language.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21281

To: davidhurka, #okular
Cc: aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen

D21281: [RFC] Write a bit Documentation for Part and PageView

[David Hurka]

davidhurka updated this revision to Diff 58857.
davidhurka added a comment.


  - Add structure to open*(url) methods documentation, shown in Detailed 
Description of Part
  - Make showMenu() documentation more generic.

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21281?vs=58664=58857

BRANCH
  create-part-and-pageview-class-documentation

REVISION DETAIL
  https://phabricator.kde.org/D21281

AFFECTED FILES
  core/document.h
  part.cpp
  part.h

To: davidhurka, #okular
Cc: aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen

[okular] [Bug 407998] Okular scales down pages when printing

[Nate Graham]

https://bugs.kde.org/show_bug.cgi?id=407998

--- Comment #8 from Nate Graham  ---
Darn. That would be a good bug to file against Neon | Snaps.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 407998] Okular scales down pages when printing

[Sergio]

https://bugs.kde.org/show_bug.cgi?id=407998

--- Comment #7 from Sergio  ---
Wanted to give 1.7.x a try and tried installing the snap. That does not work,
though.


t.qpa.xcb: could not connect to display :0
qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it
was found.
This application failed to start because no Qt platform plugin could be
initialized. Reinstalling the application may fix this problem.

Available platform plugins are: eglfs, linuxfb, minimal, minimalegl, offscreen,
vnc, wayland-egl, wayland, wayland-xcomposite-egl, wayland-xcomposite-glx, xcb.

Aborted (core dumped)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 408075] New: Okular flips pages if history is exhausted while using the back / forward keyboard shortcuts

[Kishore Gopalakrishnan]

https://bugs.kde.org/show_bug.cgi?id=408075

Bug ID: 408075
   Summary: Okular flips pages if history is exhausted while using
the back / forward keyboard shortcuts
   Product: okular
   Version: 1.7.1
  Platform: Other
OS: Linux
Status: REPORTED
  Severity: normal
  Priority: NOR
 Component: general
  Assignee: okular-devel@kde.org
  Reporter: kishor...@gmail.com
  Target Milestone: ---

SUMMARY
Okular binds the alt+shift+left/right shortcuts to the 'back' and 'forward'
items by default. If the history is exhausted (i.e. we navigate back/forward
till there are no more locations left), okular starts moving to the
next/previous page, acting as if we have pressed just the left/right arrow key.

STEPS TO REPRODUCE
1. Open a PDF in Okular.
2. Add the back and forward buttons to the toolbar (this is just to illustrate
the issue)
3. Note that the 'forward' button is greyed out.
4. Press Alt+Shift+Right (forward). 
5. Note that Okular moves forward one page.
6. Now scroll up and down to populate the history.
7. Press Alt+Shift+Left until the history of previous locations is exhausted.
At this point, the 'back' button will be greyed out and unclickable.
8. Press Alt+Shift+Left
8. Note that this causes Okular to move one page back (as if we had just
pressed the left arrow key)

OBSERVED RESULT
Okular flips pages when there is nowhere to go to in the history.

EXPECTED RESULT
Okular should do nothing if history is exhausted (i.e. after steps 8 and 4).

SOFTWARE/OS VERSIONS
Linux distro: Arch Linux
KDE Plasma Version: 5.15.90
KDE Frameworks Version: 5.58.0
Qt Version: 5.13.0

ADDITIONAL INFORMATION
This is reproducible only if the shortcut contains an arrow key. If we bind
something like ctrl+9 to 'forward', Okular shows the expected behaviour. Prima
facie, it looks like Okular doesn't correctly handle the keypresses
corresponding to a shortcut if the shortcut corresponds to an action that
cannot be performed (like going 'forward' when we are at the most recently
veiwed location).

-- 
You are receiving this mail because:
You are the assignee for the bug.

D10859: Improve layout of annotation configuraton dialogs

[Rajeesh K Nambiar]

knambiar added a comment.


  Tested, LGTM.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D10859

To: simgunz, #okular, #vdg, ngraham
Cc: aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, darcyshen

[okular] [Bug 408056] Activating edit tool automatically

[Martin]

https://bugs.kde.org/show_bug.cgi?id=408056

--- Comment #2 from Martin  ---
Thank you Oliver! Good to know it exist :)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 408056] Activating edit tool automatically

[Oliver Sander]

https://bugs.kde.org/show_bug.cgi?id=408056

Oliver Sander  changed:

   What|Removed |Added

 CC||oliver.san...@tu-dresden.de
 Status|REPORTED|RESOLVED
 Resolution|--- |INTENTIONAL

--- Comment #1 from Oliver Sander  ---
Use double-click.

Admittedly, that's quite un-discoverable.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 408056] New: Activating edit tool automatically

[Martin]

https://bugs.kde.org/show_bug.cgi?id=408056

Bug ID: 408056
   Summary: Activating edit tool automatically
   Product: okular
   Version: 1.5.3
  Platform: Other
OS: Linux
Status: REPORTED
  Severity: wishlist
  Priority: NOR
 Component: general
  Assignee: okular-devel@kde.org
  Reporter: martin.marmso...@gmail.com
  Target Milestone: ---

Every time using the "rezesion" tool (F6) it must be clicked on the toolbar to
select the appropriate tool needed, even it is used multiple times. Would it be
possible to make an option, so the tool isn't deselected when editing the pdf?
For example for the marker it would be nice not having to click every time on
the toolbar after marking a piece of text.

For pop-up notes or inline notes I think it depends if somebody whishes to have
it always in hand. So maybe making an option for every single tool?

-- 
You are receiving this mail because:
You are the assignee for the bug.

D10859: Improve layout of annotation configuraton dialogs

[Simone Gaiarin]

simgunz added a comment.


  Using the QFormLayout simplified the code a quite a bit!
  
  Regarding the stamp annotation tool, I have aligned the combobox to the top, 
because I have not found a way to vertically align the label. Though I will 
soon open a new review with some changes for that annotation tool in 
particular, where I plan to move the preview beneath the combobox. (or maybe I 
can do it directly in this review?)
  
  F6857731: Screenshot_20190529_095154.png 


INLINE COMMENTS

> annotationwidgets.cpp:206
> +formlayout->setLabelAlignment( Qt::AlignRight );
> +formlayout->setFieldGrowthPolicy( QFormLayout::AllNonFixedFieldsGrow );
>  if ( hasColorButton() )

Is it correct to set this growth policy? I have not found any guideline on the 
HIG. Compared to the grid layout where label and widget take the same amount of 
horizontal space, now the widgets take way more space making it a little ugly 
for some annotation tools. Any idea?

F6857727: Screenshot_20190529_095013.png 

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D10859

To: simgunz, #okular, #vdg, ngraham
Cc: aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, darcyshen

D10859: Improve layout of annotation configuraton dialogs

[Simone Gaiarin]

simgunz updated this revision to Diff 58817.
simgunz added a comment.


  - Use form layout instead of grid layout
  - Set label alignment and field growth policy
  - Resort code of line annotation
  - Fix wrong widget set as parent
  - Move line termination styles before leaders style
  - Align combobox to top

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10859?vs=58732=58817

BRANCH
  fix-annot-config-dialog

REVISION DETAIL
  https://phabricator.kde.org/D10859

AFFECTED FILES
  ui/annotationpropertiesdialog.cpp
  ui/annotationwidgets.cpp
  ui/annotationwidgets.h

To: simgunz, #okular, #vdg, ngraham
Cc: aacid, okular-devel, knambiar, ngraham, joaonetto, tfella, darcyshen