Comment 3 for bug 1912419

Revision history for this message
Adam Reichold (adamreichold) wrote :

Hello,

As discussed earlier, I would very much prefer using Launchpad's code review facilities to make this easier to discuss. The main issue I see here is that I do not follow how

> On macOS presentation view becomes a variant of full screen.

is implied by the other observations made above. More concretely, I would prefer to not entangle those two concepts, especially as macOS is relatively niche platform from qpdfview's point of view.

There are various small nits on the code level but as written initially that would be easier to review using Launchpad's merge requests. It would certainly benefit from splitting out the bug fixes from the behaviour changes. I would also suggest trying to avoid the duplicate book keeping between main window and document view, e.g. by returning the newly created presentation view widget from the DocumentView::startPresentation method instead of holding on to it internally.

Regards,
Adam